docs: correct observability and optimization guides - #1070
Open
miyoungc wants to merge 2 commits into
Open
Conversation
miyoungc
force-pushed
the
codex/fix-observability-experiment-docs
branch
from
August 4, 2026 16:35
a05bb30 to
70bf357
Compare
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
miyoungc
force-pushed
the
codex/fix-observability-experiment-docs
branch
from
August 4, 2026 16:40
70bf357 to
86a3471
Compare
miyoungc
marked this pull request as ready for review
August 4, 2026 16:44
Contributor
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughChangesDocumentation workflows
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/agents/insight-driven-optimization.mdx (1)
160-179: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDocument the lifecycle of Insight evidence.
trace_refsremain persisted after the 90-day TTL removes their spans and trace-index rows. The evidence table omits unavailable traces and reports how many could not be loaded. Update both guides to describe this behavior for platform Insights and local YAML files.🤖 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 `@docs/agents/insight-driven-optimization.mdx` around lines 160 - 179, Update the Insight evidence lifecycle documentation in docs/agents/insight-driven-optimization.mdx (lines 160-179) and docs/agents/observability.mdx (lines 362-363) to state that trace_refs remain persisted after the 90-day TTL removes their spans and trace-index rows, while unavailable traces are omitted from the evidence table and the number of omitted traces is reported; describe this behavior for both platform Insights and local YAML files.
🤖 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 `@docs/agents/insight-driven-optimization.mdx`:
- Around line 223-235: Update the dataset descriptions in the Experimenter
workflow around the validation and training evaluation steps (Lines 126–128) so
they reflect that survivors are first ranked by validation rewards, then
evaluated on the training split. Ensure the documented ordering matches the
implementation and does not describe repeated selection based on held-out
validation scores.
- Around line 428-434: Update the Experimenter evaluation-results description in
docs/agents/insight-driven-optimization.mdx (428-434) to state that each trial
is retrieved by trace_id and every evaluator result is attached to
trace.root_span_id with session_id=trace.session_id, replacing the generic
“attached to the trace” wording. In docs/agents/observability.mdx (254-272),
distinguish this explicit API attachment path from the ATIF-synthesized
harbor.verifier span, with no other behavioral changes.
In `@docs/agents/observability.mdx`:
- Around line 98-108: Update docs/agents/observability.mdx lines 98-108 to use
NMP_BASE_URL=http://localhost:8080 and add the required existing-instance and
/health/ready preflight before CLI/API operations; update lines 124-126 to start
services only when no ready instance exists. In
docs/agents/insight-driven-optimization.mdx lines 299-336, verify /health/ready
before running token and agent commands, preserving the documented
authentication behavior.
In `@docs/evaluator/experiments.mdx`:
- Line 87: Update docs/evaluator/experiments.mdx at lines 87-87 to define each
session without a test_case_id as its own test case and include it in
test_case_count and rollups; at lines 249-252, describe test_case_id as grouping
repeated sessions into one test case without requiring it for rollups; and at
lines 504-504, remove missing test_case_id from the causes of zero metrics.
- Around line 126-131: Update the local setup sections around the environment
exports and CLI examples to use NMP_BASE_URL=http://localhost:8080, check
whether an existing instance is available, and direct users to SETUP.md when it
is not ready. Add a /health/ready verification before running nemo auth token or
other API commands, while keeping the deployed endpoint configurable in a
separate setup branch and applying the same sequence to the referenced sections.
- Around line 126-136: Update the authentication examples around NMP_BASE_URL
and NMP_ACCESS_TOKEN to require a trusted https:// base URL whenever the bearer
token is used. Keep http:// examples only for deployments with authentication
disabled, where the Authorization header and NMP_ACCESS_TOKEN are omitted.
---
Outside diff comments:
In `@docs/agents/insight-driven-optimization.mdx`:
- Around line 160-179: Update the Insight evidence lifecycle documentation in
docs/agents/insight-driven-optimization.mdx (lines 160-179) and
docs/agents/observability.mdx (lines 362-363) to state that trace_refs remain
persisted after the 90-day TTL removes their spans and trace-index rows, while
unavailable traces are omitted from the evidence table and the number of omitted
traces is reported; describe this behavior for both platform Insights and local
YAML files.
🪄 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: ced3c009-b19a-4d96-b457-3942b1997282
📒 Files selected for processing (3)
docs/agents/insight-driven-optimization.mdxdocs/agents/observability.mdxdocs/evaluator/experiments.mdx
Comment on lines
+223
to
235
| 1. **Baseline**: Build the baseline agent (`agent-0`) and evaluate it on the | ||
| validation split; build an initial goal tree (a weighted capability rubric | ||
| used for trajectory scoring). | ||
| 2. **Analyze** — read the target Insight and perform root cause analysis. | ||
| 3. **Propose** — generate a small number of candidate improvements targeting | ||
| 2. **Analyze**: Read the target Insight and perform root cause analysis. | ||
| 3. **Propose**: Generate a small number of candidate improvements targeting | ||
| those root causes, each tagged with an optimization type. | ||
| 4. **Implement** — a coding agent applies each proposed change to a copy of | ||
| the agent and runs an integration smoke test with a bounded repair loop. | ||
| 5. **Validate** — score new candidates on the held-out validation split, | ||
| 4. **Implement**: A coding agent applies each proposed change to a copy of | ||
| the agent and runs a basic integration test with a bounded repair loop. | ||
| 5. **Validate**: Score new candidates on the held-out validation split, | ||
| optionally adding a qualitative trajectory score against the goal tree. | ||
| 6. **Select and continue** — keep a diverse Pareto front of survivors and | ||
| 6. **Select and continue**: Keep a diverse Pareto front of survivors and | ||
| iterate until a budget or convergence condition is met, then pick the | ||
| winner. |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file=$(fd -t f -p 'docs/agents/insight-driven-optimization.mdx' | head -n 1)
test -n "$file"
sed -n '100,145p;215,240p' "$file"
printf '\n--- related dataset terminology ---\n'
rg -n -i 'train|validation|held-out|test|select|surviv|rank|score|baseline|candidate' "$file"Repository: NVIDIA-NeMo/nemo-platform
Length of output: 8495
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- optimizer implementation files ---'
rg -l -i 'Pareto|surviv|validation.*split|train.*split|held.?out|Select and continue|candidate.*score' --glob '!docs/agents/insight-driven-optimization.mdx' . | head -80
printf '%s\n' '--- optimizer-related paths ---'
git ls-files | rg -i 'optimizer|experiment|harbor|insight' | head -120
printf '%s\n' '--- isolation and evaluation documentation ---'
sed -n '245,275p;410,475p' docs/agents/insight-driven-optimization.mdxRepository: NVIDIA-NeMo/nemo-platform
Length of output: 19897
🏁 Script executed:
#!/bin/bash
set -eu
for f in \
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/loop.py \
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/base.py \
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py \
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/holdout_utils.py \
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/dataset_staging.py
do
printf '\n--- %s ---\n' "$f"
ast-grep outline "$f"
doneRepository: NVIDIA-NeMo/nemo-platform
Length of output: 7780
🏁 Script executed:
#!/bin/bash
set -eu
f=plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/loop.py
rg -n '^ (async )?def (_evaluate_validation_candidates|_evaluate_insight_candidates|_evaluate_and_persist_insight_candidates|_select_survivors|_evaluate_train_candidates|run|_run|_finalize|_evaluate_agent)' "$f"Repository: NVIDIA-NeMo/nemo-platform
Length of output: 639
🏁 Script executed:
#!/bin/bash
set -eu
f=plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/loop.py
sed -n '360,470p;1300,1615p;1840,1935p' "$f"Repository: NVIDIA-NeMo/nemo-platform
Length of output: 22611
🏁 Script executed:
#!/bin/bash
set -eu
f=plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/loop.py
rg -n '(_evaluate_validation_candidates|_evaluate_train_candidates|_select_survivors|record_reward\(|val_reward|train_reward)' "$f"
sed -n '470,760p;1610,1845p' "$f"Repository: NVIDIA-NeMo/nemo-platform
Length of output: 24514
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import ast
from pathlib import Path
path = Path("plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/loop.py")
tree = ast.parse(path.read_text())
optimizer = next(
node for node in ast.walk(tree)
if isinstance(node, ast.ClassDef) and node.name == "EvolutionaryOptimizer"
)
methods = {
node.name: node
for node in optimizer.body
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef))
}
select_source = ast.get_source_segment(path.read_text(), methods["_select_survivors"])
assert 'c.reward("validation")' in select_source
assert 'pareto_sort' in select_source
run_source = ast.get_source_segment(path.read_text(), methods["_run"])
select_pos = run_source.index("self._select_survivors")
train_pos = run_source.index("self._evaluate_train_candidates")
assert select_pos < train_pos
print("survivor ranking: validation reward")
print("round order: select survivors before evaluate train candidates")
PYRepository: NVIDIA-NeMo/nemo-platform
Length of output: 262
Align the dataset descriptions.
The Experimenter ranks survivors with validation rewards before evaluating survivors on the training split. Update Lines 126–128 to match this behavior. Repeated selection on validation scores can overfit the held-out split.
🧰 Tools
🪛 LanguageTool
[style] ~227-~227: Specify a number, remove phrase, use “a few”, or use “some”
Context: ...ause analysis. 3. Propose: Generate a small number of candidate improvements targeting tho...
(SMALL_NUMBER_OF)
🤖 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 `@docs/agents/insight-driven-optimization.mdx` around lines 223 - 235, Update
the dataset descriptions in the Experimenter workflow around the validation and
training evaluation steps (Lines 126–128) so they reflect that survivors are
first ranked by validation rewards, then evaluated on the training split. Ensure
the documented ordering matches the implementation and does not describe
repeated selection based on held-out validation scores.
Comment on lines
+428
to
434
| - *How evaluation scores reach Intake.* The Analyst and the Experimenter both | ||
| read scores from Intake, but neither Harbor nor the verifier pushes scores | ||
| there; the verifier's reward is written to local disk by default. The | ||
| Experimenter uploads scores to Intake after the Harbor run completes, and | ||
| each verifier result is stored as an `evaluation_results` record attached to | ||
| the trace in Intake. Pushing results to Intake requires a platform client, | ||
| so pass `--base-url` and `--workspace`. |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- cited sections ---'
sed -n '400,450p' docs/agents/insight-driven-optimization.mdx
sed -n '235,310p' docs/agents/observability.mdx
printf '%s\n' '--- related references ---'
rg -n -C 3 'evaluation_results|evaluator result|verifier_result|Experimenter|span_id|trace_id|harbor\.verifier|reward\.json|evaluation_results' docs src .github 2>/dev/null | head -n 500Repository: NVIDIA-NeMo/nemo-platform
Length of output: 32249
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- repository candidates ---'
git ls-files | rg '(^|/)(experimentalist|experimenter|optimizer|intake|evaluator|harbor|observability)' | head -n 300
printf '%s\n' '--- upload and identifier references ---'
rg -n -C 4 'evaluator-results|evaluation_results|span_id|trace_id|session_id|harbor\.verifier|reward\.json|verifier_result' --glob '*.py' --glob '*.ts' --glob '*.tsx' --glob '*.go' --glob '*.yaml' --glob '*.json' . | head -n 800Repository: NVIDIA-NeMo/nemo-platform
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -eu
for f in \
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py \
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/base.py \
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/reporting.py \
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/experiment_mirror.py \
plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor.py \
plugins/nemo-experimentalist/tests/experimentalist/test_reporting.py
do
if [ -f "$f" ]; then
printf '\n--- %s (%s lines) ---\n' "$f" "$(wc -l < "$f")"
ast-grep outline "$f" 2>/dev/null || true
fi
done
printf '\n--- focused source references ---\n'
rg -n -C 6 'evaluator|result|span|trace|session|intake|reward|Harbor' \
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py \
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/base.py \
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/reporting.py \
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/experiment_mirror.py \
plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor.py \
plugins/nemo-experimentalist/tests/experimentalist/test_reporting.pyRepository: NVIDIA-NeMo/nemo-platform
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- exact symbols in Experimenter package ---'
rg -n 'publish|upload|evaluator.?result|evaluation_results|Intake|intake|span_id|trace_id|session_id|trace artifact|harbor\.verifier' \
plugins/nemo-experimentalist/src plugins/nemo-experimentalist/tests \
--glob '*.py' | head -n 300
printf '%s\n' '--- HarborEvaluator ranges ---'
sed -n '1235,1345p' plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py
printf '%s\n' '--- reporting outline/content ---'
wc -l plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/reporting.py
sed -n '1,260p' plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/reporting.py
printf '%s\n' '--- mirror outline/content ---'
wc -l plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/experiment_mirror.py
sed -n '1,300p' plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/experiment_mirror.pyRepository: NVIDIA-NeMo/nemo-platform
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- backend upload implementation ---'
sed -n '560,715p' plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/experimentalist_backend.py
printf '%s\n' '--- backend tests ---'
rg -n -C 8 'evaluator_results|root_span_id|session_id|trace_id|reward|upload' \
plugins/nemo-experimentalist/tests --glob '*backend*.py' --glob '*.py' | head -n 500
printf '%s\n' '--- relevant backend test files ---'
git ls-files plugins/nemo-experimentalist/tests | rg 'backend|report|mirror|loop'Repository: NVIDIA-NeMo/nemo-platform
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
backend = Path("plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/experimentalist_backend.py").read_text()
insight = Path("docs/agents/insight-driven-optimization.mdx").read_text()
observability = Path("docs/agents/observability.mdx").read_text()
openapi = Path("openapi/ga/openapi.yaml").read_text()
required_backend = [
"trace_id = uri.removeprefix(\"intake://traces/\")",
"trace = await self._retrieve_trace_with_retry(trace_id, workspace=workspace)",
"span_id=trace.root_span_id",
"session_id=trace.session_id",
"name=name",
"data_type=\"NUMERIC\"",
]
for fragment in required_backend:
assert fragment in backend, fragment
assert "attached to\nthe trace in Intake" in insight
assert "named `harbor.verifier`" in observability
assert '"span_id": "<span-id>"' in observability
assert "List Evaluator Results For Span" in openapi
assert "required:\n - span_id\n - session_id" in openapi
print("Experimenter targets the retrieved trace root span and requires session_id; docs currently use trace-level wording while observability documents span-level access.")
PYRepository: NVIDIA-NeMo/nemo-platform
Length of output: 252
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
backend = Path("plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/experimentalist_backend.py").read_text()
insight = Path("docs/agents/insight-driven-optimization.mdx").read_text()
observability = Path("docs/agents/observability.mdx").read_text()
openapi = Path("openapi/ga/openapi.yaml").read_text()
checks = {
"trace URI lookup": 'trace_id = uri.removeprefix("intake://traces/")' in backend,
"trace retrieval": 'trace = await self._retrieve_trace_with_retry(trace_id, workspace=workspace)' in backend,
"root span target": 'span_id=trace.root_span_id' in backend,
"session target": 'session_id=trace.session_id' in backend,
"metric name": 'name=name' in backend,
"numeric type": 'data_type="NUMERIC"' in backend,
"Insight trace wording": "attached to\nthe trace in Intake" in insight,
"ATIF verifier span wording": "named `harbor.verifier`" in observability,
"span example": '"span_id": "<span-id>"' in observability,
"span results endpoint": "List Evaluator Results For Span" in openapi,
"required target fields": "required:\n - span_id\n - session_id" in openapi,
}
for name, passed in checks.items():
print(f"{'PASS' if passed else 'FAIL'}: {name}")
for label, text, needle in [
("Insight", insight, "evaluation_results"),
("backend", backend, "root_span_id"),
("OpenAPI", openapi, "EvaluatorResultInput"),
]:
print(f"\n--- {label} matches ---")
lines = text.splitlines()
for i, line in enumerate(lines, 1):
if needle in line:
print(f"{i}: {line}")
PYRepository: NVIDIA-NeMo/nemo-platform
Length of output: 778
Document the Experimenter’s root-span attachment. It retrieves each trial by trace_id and attaches every evaluator result to trace.root_span_id with session_id=trace.session_id. Replace “attached to the trace” in docs/agents/insight-driven-optimization.mdx and distinguish this explicit API path from the ATIF-synthesized harbor.verifier span in docs/agents/observability.mdx.
📍 Affects 2 files
docs/agents/insight-driven-optimization.mdx#L428-L434(this comment)docs/agents/observability.mdx#L254-L272
🤖 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 `@docs/agents/insight-driven-optimization.mdx` around lines 428 - 434, Update
the Experimenter evaluation-results description in
docs/agents/insight-driven-optimization.mdx (428-434) to state that each trial
is retrieved by trace_id and every evaluator result is attached to
trace.root_span_id with session_id=trace.session_id, replacing the generic
“attached to the trace” wording. In docs/agents/observability.mdx (254-272),
distinguish this explicit API attachment path from the ATIF-synthesized
harbor.verifier span, with no other behavioral changes.
Comment on lines
+98
to
+108
| ```bash | ||
| export NMP_BASE_URL=http://127.0.0.1:8080 | ||
| export WORKSPACE=default | ||
| export NMP_STUDIO_URL="$NMP_BASE_URL/studio" | ||
| export NMP_ACCESS_TOKEN="$(nemo auth token)" | ||
| ``` | ||
|
|
||
| Set `NMP_STUDIO_URL` explicitly if Studio uses a different origin or path. | ||
| The examples use bearer-token authentication and stop on HTTP errors. For a local development | ||
| deployment with authentication disabled, omit the `Authorization` header and the | ||
| `NMP_ACCESS_TOKEN` command. Also omit `OTEL_EXPORTER_OTLP_HEADERS` from the OTLP example. |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add the required local readiness preflight.
The local workflows invoke CLI and API operations without checking for an existing instance and /health/ready. Line 99 also uses 127.0.0.1, but the required base URL is http://localhost:8080.
docs/agents/observability.mdx#L98-L108: Set the required base URL and add the preflight.docs/agents/observability.mdx#L124-L126: Start services only when no ready instance exists.docs/agents/insight-driven-optimization.mdx#L299-L336: Verify readiness before token and agent commands.
As per coding guidelines, set NMP_BASE_URL=http://localhost:8080, check for an existing instance, and verify /health/ready before local CLI operations.
📍 Affects 2 files
docs/agents/observability.mdx#L98-L108(this comment)docs/agents/observability.mdx#L124-L126docs/agents/insight-driven-optimization.mdx#L299-L336
🤖 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 `@docs/agents/observability.mdx` around lines 98 - 108, Update
docs/agents/observability.mdx lines 98-108 to use
NMP_BASE_URL=http://localhost:8080 and add the required existing-instance and
/health/ready preflight before CLI/API operations; update lines 124-126 to start
services only when no ready instance exists. In
docs/agents/insight-driven-optimization.mdx lines 299-336, verify /health/ready
before running token and agent commands, preserving the documented
authentication behavior.
Source: Coding guidelines
| | Rollup | Meaning | | ||
| |--------|---------| | ||
| | `test_case_count` | Number of distinct test cases (distinct non-empty `test_case_id` values). Sessions with no `test_case_id` don't count toward it or the rollups. | | ||
| | `test_case_count` | Number of distinct test cases (distinct non-empty `test_case_id` values). Sessions with no `test_case_id` do not count toward it or the rollups. | |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Correct missing-test_case_id semantics.
The Evaluation schema counts each session without test_case_id as its own test case. It includes those sessions in test_case_count and rollups.
docs/evaluator/experiments.mdx#L87-L87: Define missing IDs as one test case per session, not excluded sessions.docs/evaluator/experiments.mdx#L249-L252: State thattest_case_idgroups repeated sessions into one test case; do not state that it is required for rollups.docs/evaluator/experiments.mdx#L504-L504: Remove missingtest_case_idas a zero-metric cause.
The supplied openapi/ga/openapi.yaml Evaluation schema defines this behavior.
📍 Affects 1 file
docs/evaluator/experiments.mdx#L87-L87(this comment)docs/evaluator/experiments.mdx#L249-L252docs/evaluator/experiments.mdx#L504-L504
🤖 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 `@docs/evaluator/experiments.mdx` at line 87, Update
docs/evaluator/experiments.mdx at lines 87-87 to define each session without a
test_case_id as its own test case and include it in test_case_count and rollups;
at lines 249-252, describe test_case_id as grouping repeated sessions into one
test case without requiring it for rollups; and at lines 504-504, remove missing
test_case_id from the causes of zero metrics.
Comment on lines
+126
to
131
| ```bash | ||
| export NMP_BASE_URL=http://127.0.0.1:8080 | ||
| export WORKSPACE=default | ||
| export NMP_STUDIO_URL="$NMP_BASE_URL/studio" | ||
| export NMP_ACCESS_TOKEN="$(nemo auth token)" | ||
| ``` |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the local setup sequence.
For the local flow, use NMP_BASE_URL=http://localhost:8080 and check for an existing instance. If no instance is ready, follow SETUP.md to start it. Verify /health/ready before nemo auth token and the API commands. Keep the deployed endpoint configurable in a separate branch.
As per coding guidelines, follow SETUP.md, check for an existing instance, and verify /health/ready before local CLI operations.
Also applies to: 151-156, 168-181
🤖 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 `@docs/evaluator/experiments.mdx` around lines 126 - 131, Update the local
setup sections around the environment exports and CLI examples to use
NMP_BASE_URL=http://localhost:8080, check whether an existing instance is
available, and direct users to SETUP.md when it is not ready. Add a
/health/ready verification before running nemo auth token or other API commands,
while keeping the deployed endpoint configurable in a separate setup branch and
applying the same sequence to the referenced sections.
Source: Coding guidelines
Comment on lines
+126
to
+136
| ```bash | ||
| export NMP_BASE_URL=http://127.0.0.1:8080 | ||
| export WORKSPACE=default | ||
| export NMP_STUDIO_URL="$NMP_BASE_URL/studio" | ||
| export NMP_ACCESS_TOKEN="$(nemo auth token)" | ||
| ``` | ||
|
|
||
| Set `NMP_STUDIO_URL` explicitly if Studio uses a different origin or path. | ||
| The examples use bearer-token authentication and stop on HTTP errors. For a local development | ||
| deployment with authentication disabled, omit the `Authorization` header and the | ||
| `NMP_ACCESS_TOKEN` command. |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '110,185p' docs/evaluator/experiments.mdxRepository: NVIDIA-NeMo/nemo-platform
Length of output: 3208
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
text = Path("docs/evaluator/experiments.mdx").read_text()
assert "export NMP_BASE_URL=http://127.0.0.1:8080" in text
assert 'export NMP_ACCESS_TOKEN="$(nemo auth token)"' in text
assert '-H "Authorization: Bearer $NMP_ACCESS_TOKEN"' in text
print("The documented default uses HTTP while sending the bearer token.")
PYRepository: NVIDIA-NeMo/nemo-platform
Length of output: 228
Sensitive Data Exposure (CWE-319): Cleartext Transmission of Sensitive Information
Reachability: External
Require HTTPS for token-authenticated requests.
Use HTTP only when authentication is disabled. Require a trusted https:// base URL whenever $NMP_ACCESS_TOKEN is sent.
🤖 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 `@docs/evaluator/experiments.mdx` around lines 126 - 136, Update the
authentication examples around NMP_BASE_URL and NMP_ACCESS_TOKEN to require a
trusted https:// base URL whenever the bearer token is used. Keep http://
examples only for deployments with authentication disabled, where the
Authorization header and NMP_ACCESS_TOKEN are omitted.
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
Author
CLI and API example verificationPushed commit
I also rechecked the remaining command surfaces against their Typer registrations and implementation code:
Validation results:
Verification was source-based because this checkout does not contain a synced |
Contributor
|
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
Why
This is a documentation correctness follow-up to #1048 and #1014. Several examples and behavioral descriptions no longer matched the merged implementation, and remote-deployment examples assumed unauthenticated localhost defaults.
Root cause
The guides described intended behavior while the Intake rollup, deletion, and optimizer command implementations continued to evolve. The examples also did not account for curl URL globbing, authenticated deployments, or the Analyst's profile-based local persistence mode.
Impact
Readers can now run the documented commands against authenticated local or remote deployments and verify the correct persistence target. The Experiments and optimization behavior described in the guides matches the current checked-in implementation.
Validation
make docs-check(215 MDX files parsed cleanly; gated-link validation passed)bash -ngit diff --checkmake docs-broken-linksreported no issues in the modified pages; four existing errors remain in unrelated authentication and Studio pagesSummary by CodeRabbit