feat(optimization): Agents-owned Fabric Optuna HPO with Hermes + MCP (AALGO-277) - #608
Merged
Conversation
soluwalana
changed the title
soluwalana
force-pushed
the
aalgo-277/solu
branch
from
August 4, 2026 18:40
ab8f69d to
397a8ff
Compare
Contributor
|
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-608-aalgo-277-solu.docs.buildwithfern.com/nemo-platform |
soluwalana
force-pushed
the
aalgo-277/solu
branch
from
August 4, 2026 22:26
4ca4fef to
801a093
Compare
soluwalana
marked this pull request as ready for review
August 4, 2026 22:27
soluwalana
changed the title
soluwalana
force-pushed
the
aalgo-277/solu
branch
from
August 4, 2026 22:31
801a093 to
f8a7aa6
Compare
Contributor
|
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:
📝 WalkthroughWalkthroughThe pull request adds Fabric-native numeric optimization with Optuna, task-scoped Fabric hooks, MCP bindings, tunable RAG evaluation, updated job wiring, Docker handling, examples, tests, and migration documentation. ChangesFabric optimization platform
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 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: 8
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (14)
docs/agents/index.mdx-57-63 (1)
57-63: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove
optimizefrom the NAT-only table.
nemo agents optimize runnow requires a Fabric-native package. This row directs users to submit NAT tuning configurations that the job rejects. Keepevaluateas legacy, or describeoptimizeas Fabric-backed.🤖 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/index.mdx` around lines 57 - 63, Remove the Legacy NAT-only table row for the `nemo agents optimize run` command in the documentation, leaving the `evaluate` row unchanged; do not present NAT optimization as NAT-only, since optimization now requires a Fabric-native package.plugins/nemo-agents/tests/unit/usage/test_usage_cli.py-204-220 (1)
204-220: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
chmod(0o000)does not block reads for root. Many CI containers run tests as UID 0, where the open succeeds and the command exits 0. The test then fails onexit_code == 1. Skip the test whenos.geteuid() == 0, or force the read error by patching the read path instead.Proposed guard
+@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, reason="root bypasses file mode checks") def test_usage_show_unreadable_result_json_exits_cleanly(app, tmp_path: Path) -> None:🤖 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-agents/tests/unit/usage/test_usage_cli.py` around lines 204 - 220, Update test_usage_show_unreadable_result_json_exits_cleanly to avoid relying on chmod-based unreadability when running as root: skip the test when os.geteuid() == 0, or patch the file-reading path to deterministically raise the expected read error. Preserve the existing assertions for non-root execution.plugins/nemo-optimization/README.md-16-19 (1)
16-19: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove or fix the
examples/hermes-optimize/hooks/reference. The directory does not exist in this repository, so the README link is broken.🤖 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-optimization/README.md` around lines 16 - 19, Remove the nonexistent examples/hermes-optimize/hooks/ reference from the README, or replace it with a valid repository path that documents the per-task Fabric lifecycle hooks. Keep the surrounding hook configuration references and explanation intact.plugins/nemo-optimization/src/nemo_optimization/router.py-65-68 (1)
65-68: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReturn
OptimizeRouterErrorwhen discovery misses a backend.Line 67 indexes the discovery map directly. If
optunaorgais not registered,dispatch_payload()raisesKeyError. Match the guarded.get()path indispatch()and add a missing-backend test.🤖 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-optimization/src/nemo_optimization/router.py` around lines 65 - 68, Update dispatch_payload() to retrieve the selected backend with the same guarded lookup used by dispatch(), and return OptimizeRouterError when discovery does not contain the backend instead of allowing KeyError. Add a test covering a missing optuna or ga registration and verify the expected error response.plugins/nemo-optimization/src/nemo_optimization/preflight.py-31-50 (1)
31-50: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMerge each LLM configuration before validation.
Lines 32-35 replace an agent LLM mapping when the optimize YAML overrides the same key. If the YAML only changes
llms.default.model_name,_typeis removed and Lines 43-49 skip validation.Use the same deep-merge semantics as
build_optimize_payload(). Add a test with a partial LLM override.🤖 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-optimization/src/nemo_optimization/preflight.py` around lines 31 - 50, The LLM collection in the preflight flow must deep-merge agent and optimize configurations before validation, rather than replacing entries by key. Reuse the existing merge behavior or helper used by build_optimize_payload() when combining llms, then validate the merged _type and model_name in the existing loop. Add coverage for a partial override such as llms.default.model_name that preserves the base _type and triggers validation.plugins/nemo-optimization/tests/test_selection.py-28-31 (1)
28-31: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThis assertion cannot fail.
All three input trials are on the Pareto front, so
in {(0.1, 0.9), (0.2, 0.2), (0.9, 0.1)}holds for any return value. Assert the exact trial that harmonic scoring selects.🤖 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-optimization/tests/test_selection.py` around lines 28 - 31, Update test_pick_trial_harmonic_returns_pareto_member to assert the exact values of the trial selected by harmonic scoring, rather than accepting every input trial. Determine the expected harmonic winner from the provided study data and assert that specific result.plugins/nemo-optimization/src/nemo_optimization/backends/optuna/selection.py-28-52 (1)
28-52: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
weightsis ignored forharmonicandchebyshev.
study_driver.run_numeric_studypassesweights=[metric.weight for metric in config.metrics]on every call, andharmonicis the default mode. A user who setsweightinoptimizer.eval_metricsgets it silently dropped. Log a warning, or raise when non-uniform weights arrive with a mode that cannot use them.Also move the mode check to the top of the function so an invalid mode fails before the matrix and normalization work.
🤖 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-optimization/src/nemo_optimization/backends/optuna/selection.py` around lines 28 - 52, Update the selection function around _SUPPORTED_MODES to validate mode.lower() before computing the Pareto matrix or normalization, then reject or warn when non-uniform weights are supplied to harmonic or chebyshev; ensure configured weights are not silently ignored while preserving weighted behavior for sum.plugins/nemo-optimization/src/nemo_optimization/backends/optuna/config_overlay.py-54-61 (1)
54-61: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winLines 58-61 are unreachable.
Line 56-57 pops
optimizerfromconfig.config.get("optimizer")at Line 58 therefore always returnsNone, so the nestedsearch_space/optimizable_paramscleanup never runs. Pick one behavior: remove the whole key, or keepoptimizerand strip only its tuning fields.🔧 Proposed fix (full removal, matching current effective behavior)
def strip_optimizer_only_fields(config: dict[str, Any]) -> None: """Remove optimizer metadata from a trial config artifact (in-place).""" for key in _OPTIMIZER_ONLY_TOP_LEVEL_KEYS: config.pop(key, None) - optimizer = config.get("optimizer") - if isinstance(optimizer, dict): - optimizer.pop("search_space", None) - optimizer.pop("optimizable_params", None)🤖 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-optimization/src/nemo_optimization/backends/optuna/config_overlay.py` around lines 54 - 61, Update strip_optimizer_only_fields to use one consistent optimizer-cleanup strategy: since _OPTIMIZER_ONLY_TOP_LEVEL_KEYS already removes the top-level optimizer entry, remove the now-unreachable config.get("optimizer") block and retain the existing full-removal behavior.plugins/nemo-optimization/src/nemo_optimization/backends/optuna/search_space.py-96-105 (1)
96-105: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winValidate that
lowandhighare numbers.A YAML config can supply
low: "0.1". The comparison at Line 104 then raisesTypeErroron mixed types, or compares strings lexically when both are strings. Both escape theSearchSpaceErrorcontract that callers catch.🛡️ Proposed fix
if low is None or high is None: raise SearchSpaceError( "Search space entry must define either 'values' or both 'low' and 'high'." ) + if isinstance(low, bool) or isinstance(high, bool) or not isinstance(low, (int, float)) or not isinstance(high, (int, float)): + raise SearchSpaceError( + f"Search space entry {name!r} requires numeric 'low' and 'high'; " + f"got low={low!r}, high={high!r}." + ) if low >= high:🤖 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-optimization/src/nemo_optimization/backends/optuna/search_space.py` around lines 96 - 105, Validate that low and high in the search-space parsing logic are numeric before evaluating low >= high, and raise SearchSpaceError for non-numeric values. Preserve the existing requirement that both bounds are present and ensure valid numeric bounds continue through the current range validation.plugins/nemo-optimization/tests/smoke_fabric_optimize_atif.py-130-131 (1)
130-131: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the correct variable in the assertion message.
entryis the loop variable from Lines 125-128. At Line 131 it still holds the last entry, whileatif_pathcomes fromtrace_map[0]. The failure message can name a different trace.🐛 Proposed fix
- atif_path = Path(trace_map[0]["trace_ref"]) - assert atif_path.is_file(), entry["trace_ref"] + atif_path = Path(trace_map[0]["trace_ref"]) + assert atif_path.is_file(), trace_map[0]["trace_ref"]🤖 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-optimization/tests/smoke_fabric_optimize_atif.py` around lines 130 - 131, Update the assertion message in the trace-file validation near atif_path to reference the same trace_map[0] entry used to construct atif_path, rather than the stale loop variable entry, so failures identify the correct trace.plugins/nemo-optimization/src/nemo_optimization/backends/optuna/fabric_trial.py-63-63 (1)
63-63: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard
eval.generalbefore attribute access.If
eval.generalis present but null,self._eval_config.get("general", {})returnsNoneand.get(...)raisesAttributeError._load_dataset_rowsat Line 233 guards the same key withisinstance. Use the same guard here.🐛 Proposed fix
- self._parallelism = int(self._eval_config.get("general", {}).get("max_concurrency", default_parallelism)) + general = self._eval_config.get("general") + general = general if isinstance(general, Mapping) else {} + self._parallelism = int(general.get("max_concurrency", default_parallelism))🤖 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-optimization/src/nemo_optimization/backends/optuna/fabric_trial.py` at line 63, Update the parallelism initialization in the trial setup to validate that the value from self._eval_config.get("general") is a mapping before calling .get("max_concurrency"). Match the isinstance-based guard used by _load_dataset_rows, and fall back to default_parallelism when eval.general is null or otherwise invalid.plugins/nemo-optimization/tests/smoke_fabric_optimize_atif.py-86-86 (1)
86-86: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd
pytest-timeoutto the plugin'sdevdependency group. The rootpytest.inienables--strict-markers, so this test fails when the package-specific environment omits the plugin.🤖 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-optimization/tests/smoke_fabric_optimize_atif.py` at line 86, Add pytest-timeout to the plugin's dev dependency group so the `@pytest.mark.timeout` marker in smoke_fabric_optimize_atif.py is available in the package-specific environment under the root strict-marker configuration.packages/nemo_evaluator_sdk/tests/metrics/test_tunable_rag_evaluator.py-26-29 (1)
26-29: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAllow string payloads in
_judge_response.Line 116 passes a
str, which conflicts withdict[str, Any]. Usedict[str, Any] | str.🤖 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_evaluator_sdk/tests/metrics/test_tunable_rag_evaluator.py` around lines 26 - 29, Update the _judge_response parameter annotation to accept either dict[str, Any] or str, while preserving its existing JSON serialization and response structure.Source: Coding guidelines
packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_hook_loading.py-57-57 (1)
57-57: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the temporary
__init__.pyfixtures.
packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_hook_loading.py#L57-L57: Remove the write call. Importauthor_hooks.hookas a namespace package.packages/nemo_evaluator_sdk/tests/agent_eval/test_mcp_run_binding_hook.py#L245-L245: Remove the write call. Importagent_pkg.auditas a namespace package.As per coding guidelines, “Do not add
__init__.pyfiles to Python packages; prefer implicit namespace packages.”🤖 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_evaluator_sdk/tests/agent_eval/test_fabric_hook_loading.py` at line 57, Remove the temporary __init__.py write call in packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_hook_loading.py at lines 57-57 and update the related import to use author_hooks.hook as an implicit namespace package; likewise remove the write call in packages/nemo_evaluator_sdk/tests/agent_eval/test_mcp_run_binding_hook.py at lines 245-245 and import agent_pkg.audit as an implicit namespace package. Do not add replacement __init__.py files.Source: Coding guidelines
🧹 Nitpick comments (14)
plugins/nemo-optimization/README.md (1)
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the internal label. "(Alt 5)" refers to an internal design alternative and gives readers no information.
Proposed edit
-Primary user surface (Alt 5): +Primary user surface:🤖 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-optimization/README.md` at line 6, Remove the internal “(Alt 5)” label from the “Primary user surface” heading in the README, leaving the user-facing heading text unchanged.plugins/nemo-optimization/examples/hermes-optimize/README.md (1)
22-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNote the risk of
--no-deps. Installinghermes-agent==0.18.2with--no-depsinto the project venv leaves its runtime requirements unresolved, so failures appear later as import errors. Add the required imports or a one-line "if imports fail, install X" note next to the confirm step.🤖 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-optimization/examples/hermes-optimize/README.md` around lines 22 - 30, Update the “hermes-agent” harness setup section in the README to explicitly warn that installing with --no-deps may leave runtime requirements unresolved, and add a concise troubleshooting note beside the verification step instructing users to install the missing dependencies if imports fail.plugins/nemo-agents/tests/unit/usage/test_usage_cli.py (1)
105-111: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePin the exit code.
assert result.exit_code in (0, 2)accepts both behaviors, so a regression in the no-args path stays green. Assert the single code the app produces today.🤖 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-agents/tests/unit/usage/test_usage_cli.py` around lines 105 - 111, Update test_usage_with_no_args_prints_help to assert the specific current exit code produced by runner.invoke(app, ["usage"]) instead of accepting both 0 and 2; keep the help-output assertion unchanged.plugins/nemo-agents/tests/unit/test_optimize_skills_analyze_only.py (1)
254-287: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssertions can pass on unrelated failures. Both tests accept any nonzero exit code and only match a substring. A crash before the guard (for example config parsing) still yields nonzero, and the substring check is the only discriminator. Pin the exit code the guard produces, and assert the error text, so a regression that moves the failure earlier is visible.
🤖 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-agents/tests/unit/test_optimize_skills_analyze_only.py` around lines 254 - 287, Strengthen test_cli_analyze_only_requires_initial_batch_flag and test_cli_analyze_only_from_config_file_requires_initial_batch by asserting the exact nonzero exit code produced by the validation guard and matching the complete expected error text, rather than accepting any nonzero result or a partial initial-batch substring. Keep the tests focused on confirming the analyze-only initial-batch guard is reached.plugins/nemo-optimization/src/nemo_optimization/registry.py (1)
10-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport
OptimizationBackendat module scope.
OptimizationBackendhas no shown import path back toregistry.py. Remove theTYPE_CHECKINGblock and use a regular import.Proposed change
-from typing import TYPE_CHECKING - -if TYPE_CHECKING: - from nemo_optimization.backends.protocol import OptimizationBackend +from nemo_optimization.backends.protocol import OptimizationBackend @@ def discover_optimization_backends() -> dict[str, OptimizationBackend]: - from nemo_optimization.backends.protocol import OptimizationBackendAs per coding guidelines, do not import types only under
TYPE_CHECKINGwhen a regular import is possible.Also applies to: 23-24
🤖 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-optimization/src/nemo_optimization/registry.py` around lines 10 - 13, Update registry.py to remove the TYPE_CHECKING-only import block and import OptimizationBackend regularly from nemo_optimization.backends.protocol at module scope, including the other referenced occurrence.Source: Coding guidelines
plugins/nemo-optimization/src/nemo_optimization/backends/optuna/backend.py (2)
44-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueParsing happens twice.
run_studycallsparse_numeric_study_config(payload["optimizer"]), andrun_numeric_studyparses the same optimizer mapping again at study_driver.py Line 171. Pass the parsedconfigintorun_numeric_study, or readmetric_namesfrom the returned result only. Duplicate parsing lets the two paths diverge if validation changes.🤖 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-optimization/src/nemo_optimization/backends/optuna/backend.py` around lines 44 - 58, Update run_study and run_numeric_study so the optimizer configuration is parsed only once: pass the already parsed config from run_study into run_numeric_study and reuse it there, including for metric handling. Remove the second parse in run_numeric_study while preserving existing validation and study execution behavior.
82-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUnannotated parameters in new modules. Both functions omit concrete type hints, so
tycannot check the callers.
plugins/nemo-optimization/src/nemo_optimization/backends/optuna/backend.py#L82-L88: annotateoutput_dir: Pathand add the return type as theTrialEvaluatorprotocol.plugins/nemo-optimization/src/nemo_optimization/backends/optuna/atif_metadata.py#L18-L18: annotategenerate_id: Callable[[], str].🤖 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-optimization/src/nemo_optimization/backends/optuna/backend.py` around lines 82 - 88, Annotate _build_trial_evaluator in plugins/nemo-optimization/src/nemo_optimization/backends/optuna/backend.py:82-88 with output_dir: Path and return type TrialEvaluator. Annotate generate_id in plugins/nemo-optimization/src/nemo_optimization/backends/optuna/atif_metadata.py:18-18 as Callable[[], str], adding or reusing the necessary imports.Source: Coding guidelines
plugins/nemo-optimization/src/nemo_optimization/backends/optuna/study_driver.py (1)
86-126: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueDuplicate
evaluator_namevalues collapse metrics silently.Two
eval_metricsentries can resolve to the samemetric_nameat Line 104.metric_namesthen contains duplicates whiledirectionskeeps both entries, so the study optimizes the same score twice under different directions. Reject duplicate resolved metric names.🤖 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-optimization/src/nemo_optimization/backends/optuna/study_driver.py` around lines 86 - 126, Update parse_numeric_study_config so each resolved MetricSpec.name, including evaluator_name fallbacks, is unique before appending it to metrics. Track previously resolved names and raise StudyDriverError when a duplicate occurs, preventing duplicate entries from reaching the study configuration.plugins/nemo-optimization/tests/test_atif_metadata.py (1)
30-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case without
row_id.
build_atif_trial_tagsomitsATIF_ROW_IDwhenrow_idis falsy. No test covers that branch, and no test covers whitespace stripping inresolve_experiment_id.🤖 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-optimization/tests/test_atif_metadata.py` around lines 30 - 37, Extend test_build_atif_trial_tags with a case where row_id is omitted or falsy, asserting ATIF_ROW_ID is absent from the returned tags. Add a separate test for resolve_experiment_id using whitespace-padded input and assert the experiment ID is stripped as expected.plugins/nemo-optimization/tests/test_search_space.py (1)
115-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a path-collision test.
suggestions_by_pathraises when two logical names share onepath. That branch is untested. Also add a case for thelow >= highrejection.🤖 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-optimization/tests/test_search_space.py` around lines 115 - 127, Add tests in test_suggestions_by_path_maps_logical_names covering two logical names mapped to the same path and asserting suggestions_by_path raises the expected exception, plus a search-space case with low >= high asserting the parser rejects it. Reuse the existing parse_search_space and suggestions_by_path setup and preserve the current valid mapping assertion.plugins/nemo-optimization/src/nemo_optimization/backends/optuna/config_overlay.py (1)
36-51: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueCollision detection is order-dependent.
nest_dotted_paths({"a": 2, "a.b": 1})raisesKeyError, but the reverse order{"a.b": 1, "a": 2}silently overwrites the nested dict with2. Reject the scalar-over-mapping case too if the collision must always fail.🤖 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-optimization/src/nemo_optimization/backends/optuna/config_overlay.py` around lines 36 - 51, Update nest_dotted_paths to detect collisions in both insertion orders: when assigning a leaf value, reject replacing an existing mapping, while preserving the current error for traversing through a scalar. Ensure inputs such as {"a": 2, "a.b": 1} and the reverse order consistently raise KeyError instead of overwriting data.plugins/nemo-optimization/src/nemo_optimization/backends/optuna/artifacts.py (1)
169-176: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the dead
n_metrics == 1branch.
_plot_pairwiseis only called frommaybe_write_pareto_plotsafter thelen(metric_names) < 2early return. Lines 172-173 and theif n_metrics > 1fallback at Line 176 are unreachable.🤖 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-optimization/src/nemo_optimization/backends/optuna/artifacts.py` around lines 169 - 176, Remove the unreachable single-metric handling from _plot_pairwise: delete the n_metrics == 1 axes reassignment and use direct axes[row_index][col_index] access in the plotting loop. Preserve the existing caller guard in maybe_write_pareto_plots.plugins/nemo-optimization/tests/test_fabric.py (1)
37-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a case for the schema-less rejection branch.
require_fabric_agent_confighas a second failure path for configs that are neither Fabric nor NAT shaped (must declare schema_version). No test covers it.🤖 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-optimization/tests/test_fabric.py` around lines 37 - 39, Add a test alongside test_require_fabric_agent_rejects_nat that passes a schema-less, non-Fabric/non-NAT configuration to require_fabric_agent_config and asserts FabricOptimizeError with a match for “must declare schema_version”.plugins/nemo-optimization/src/nemo_optimization/backends/optuna/fabric_trial.py (1)
48-60: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCollapse the redundant
isinstancechecks.Line 48 already normalizes
fabric_evalto aMappingor{}. Theisinstance(fabric_eval, Mapping)guards at Lines 55, 57, and 59 are always true.🤖 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-optimization/src/nemo_optimization/backends/optuna/fabric_trial.py` around lines 48 - 60, Remove the redundant isinstance(fabric_eval, Mapping) guards from the _fabric_base_dir, _timeout_s, and _capture_trajectory assignments in the surrounding initialization flow. Since fabric_eval is already normalized to a Mapping or empty dictionary, access its values directly while preserving the existing defaults.
🤖 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_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks_mcp_binding.py`:
- Around line 287-305: Update the binding setup flow in prepare so each created
binding and handoff is registered before config.add_mcp_server or any subsequent
setup that may fail, allowing self.cleanup(session) to release resources from
partially completed entries. In the cleanup path, ensure failures from an
individual binding.cleanup() or handoff.close() are isolated so cleanup
continues for all remaining entries. Add regression tests covering
rebinding/setup failure and independent cleanup failures.
In `@plugins/nemo-agents/src/nemo_agents_plugin/utils.py`:
- Around line 433-435: Update the OptimizeJob.run preflight call to pass
Path(spec.optimize_config) into preflight_validate_llm_models instead of the
loaded configuration dict, matching that function’s read_text-based interface
and allowing SDK optimization runs to reach dispatch.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/artifacts.py`:
- Around line 62-64: Update the plotting data preparation around _trial_values
and pareto_indexes so both values and Pareto indices are built in one pass over
the same trials filtered to exactly n_metrics values. Preserve the existing
value normalization and conversion behavior, append each Pareto index using the
filtered values list position, and update the corresponding call sites such as
_plot_points usage so failed or pruned trials cannot shift highlighting.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/fabric_trial.py`:
- Around line 66-85: Update the trial evaluation flow around evaluate and
apply_suggestions to convert logical suggestion names into path-keyed
suggestions via path_suggestions before applying them. Use the same
path_suggestions mapping when building optimized_config from best_trial.params
so both trial configs and the final optimized configuration update nested paths
such as models.default.temperature.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/study_driver.py`:
- Around line 232-242: Update the study completion flow before selecting
best_trial or calling pick_trial to detect when no trials completed
successfully. Count failed trials, and raise StudyDriverError with that failure
count when the completed-trial set is empty; otherwise preserve the existing
single-objective study.best_trial and multi-objective pick_trial selection
paths.
- Around line 244-245: Update the optimized-config flow around apply_suggestions
and best_trial.params to translate logical parameter names through the same
suggestions_by_path mapping used when constructing trial configs, then apply the
resulting Fabric-path suggestions to base_config before write_optimized_config.
Preserve the existing optimized output behavior while ensuring entries such as
temperature update their configured model paths.
In `@plugins/nemo-optimization/src/nemo_optimization/fabric.py`:
- Around line 63-69: In the branch where agent_config is set, validate
optimize_config with looks_like_nat_config before copying the optimizer and eval
overlays, and reject it instead of silently discarding NAT-specific keys.
Preserve the existing require_fabric_agent_config handling and overlay behavior
for valid Fabric-shaped configurations, and add a test covering a valid Fabric
agent_config combined with a NAT-shaped optimize_config.
In `@plugins/nemo-optimization/src/nemo_optimization/jobs/optimize.py`:
- Around line 107-120: Update _expand_env and the artifact serialization flow so
environment-backed secret values are not written as expanded plaintext in either
trial or optimized configuration artifacts. Preserve ${ENV_VAR} references or
redact secret-bearing fields before yaml.safe_dump, while retaining expanded
values for runtime dispatch. Add sentinel-secret coverage for both artifact
types.
---
Minor comments:
In `@docs/agents/index.mdx`:
- Around line 57-63: Remove the Legacy NAT-only table row for the `nemo agents
optimize run` command in the documentation, leaving the `evaluate` row
unchanged; do not present NAT optimization as NAT-only, since optimization now
requires a Fabric-native package.
In `@packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_hook_loading.py`:
- Line 57: Remove the temporary __init__.py write call in
packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_hook_loading.py at
lines 57-57 and update the related import to use author_hooks.hook as an
implicit namespace package; likewise remove the write call in
packages/nemo_evaluator_sdk/tests/agent_eval/test_mcp_run_binding_hook.py at
lines 245-245 and import agent_pkg.audit as an implicit namespace package. Do
not add replacement __init__.py files.
In `@packages/nemo_evaluator_sdk/tests/metrics/test_tunable_rag_evaluator.py`:
- Around line 26-29: Update the _judge_response parameter annotation to accept
either dict[str, Any] or str, while preserving its existing JSON serialization
and response structure.
In `@plugins/nemo-agents/tests/unit/usage/test_usage_cli.py`:
- Around line 204-220: Update
test_usage_show_unreadable_result_json_exits_cleanly to avoid relying on
chmod-based unreadability when running as root: skip the test when os.geteuid()
== 0, or patch the file-reading path to deterministically raise the expected
read error. Preserve the existing assertions for non-root execution.
In `@plugins/nemo-optimization/README.md`:
- Around line 16-19: Remove the nonexistent examples/hermes-optimize/hooks/
reference from the README, or replace it with a valid repository path that
documents the per-task Fabric lifecycle hooks. Keep the surrounding hook
configuration references and explanation intact.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/config_overlay.py`:
- Around line 54-61: Update strip_optimizer_only_fields to use one consistent
optimizer-cleanup strategy: since _OPTIMIZER_ONLY_TOP_LEVEL_KEYS already removes
the top-level optimizer entry, remove the now-unreachable
config.get("optimizer") block and retain the existing full-removal behavior.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/fabric_trial.py`:
- Line 63: Update the parallelism initialization in the trial setup to validate
that the value from self._eval_config.get("general") is a mapping before calling
.get("max_concurrency"). Match the isinstance-based guard used by
_load_dataset_rows, and fall back to default_parallelism when eval.general is
null or otherwise invalid.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/search_space.py`:
- Around line 96-105: Validate that low and high in the search-space parsing
logic are numeric before evaluating low >= high, and raise SearchSpaceError for
non-numeric values. Preserve the existing requirement that both bounds are
present and ensure valid numeric bounds continue through the current range
validation.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/selection.py`:
- Around line 28-52: Update the selection function around _SUPPORTED_MODES to
validate mode.lower() before computing the Pareto matrix or normalization, then
reject or warn when non-uniform weights are supplied to harmonic or chebyshev;
ensure configured weights are not silently ignored while preserving weighted
behavior for sum.
In `@plugins/nemo-optimization/src/nemo_optimization/preflight.py`:
- Around line 31-50: The LLM collection in the preflight flow must deep-merge
agent and optimize configurations before validation, rather than replacing
entries by key. Reuse the existing merge behavior or helper used by
build_optimize_payload() when combining llms, then validate the merged _type and
model_name in the existing loop. Add coverage for a partial override such as
llms.default.model_name that preserves the base _type and triggers validation.
In `@plugins/nemo-optimization/src/nemo_optimization/router.py`:
- Around line 65-68: Update dispatch_payload() to retrieve the selected backend
with the same guarded lookup used by dispatch(), and return OptimizeRouterError
when discovery does not contain the backend instead of allowing KeyError. Add a
test covering a missing optuna or ga registration and verify the expected error
response.
In `@plugins/nemo-optimization/tests/smoke_fabric_optimize_atif.py`:
- Around line 130-131: Update the assertion message in the trace-file validation
near atif_path to reference the same trace_map[0] entry used to construct
atif_path, rather than the stale loop variable entry, so failures identify the
correct trace.
- Line 86: Add pytest-timeout to the plugin's dev dependency group so the
`@pytest.mark.timeout` marker in smoke_fabric_optimize_atif.py is available in the
package-specific environment under the root strict-marker configuration.
In `@plugins/nemo-optimization/tests/test_selection.py`:
- Around line 28-31: Update test_pick_trial_harmonic_returns_pareto_member to
assert the exact values of the trial selected by harmonic scoring, rather than
accepting every input trial. Determine the expected harmonic winner from the
provided study data and assert that specific result.
---
Nitpick comments:
In `@plugins/nemo-agents/tests/unit/test_optimize_skills_analyze_only.py`:
- Around line 254-287: Strengthen
test_cli_analyze_only_requires_initial_batch_flag and
test_cli_analyze_only_from_config_file_requires_initial_batch by asserting the
exact nonzero exit code produced by the validation guard and matching the
complete expected error text, rather than accepting any nonzero result or a
partial initial-batch substring. Keep the tests focused on confirming the
analyze-only initial-batch guard is reached.
In `@plugins/nemo-agents/tests/unit/usage/test_usage_cli.py`:
- Around line 105-111: Update test_usage_with_no_args_prints_help to assert the
specific current exit code produced by runner.invoke(app, ["usage"]) instead of
accepting both 0 and 2; keep the help-output assertion unchanged.
In `@plugins/nemo-optimization/examples/hermes-optimize/README.md`:
- Around line 22-30: Update the “hermes-agent” harness setup section in the
README to explicitly warn that installing with --no-deps may leave runtime
requirements unresolved, and add a concise troubleshooting note beside the
verification step instructing users to install the missing dependencies if
imports fail.
In `@plugins/nemo-optimization/README.md`:
- Line 6: Remove the internal “(Alt 5)” label from the “Primary user surface”
heading in the README, leaving the user-facing heading text unchanged.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/artifacts.py`:
- Around line 169-176: Remove the unreachable single-metric handling from
_plot_pairwise: delete the n_metrics == 1 axes reassignment and use direct
axes[row_index][col_index] access in the plotting loop. Preserve the existing
caller guard in maybe_write_pareto_plots.
In `@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/backend.py`:
- Around line 44-58: Update run_study and run_numeric_study so the optimizer
configuration is parsed only once: pass the already parsed config from run_study
into run_numeric_study and reuse it there, including for metric handling. Remove
the second parse in run_numeric_study while preserving existing validation and
study execution behavior.
- Around line 82-88: Annotate _build_trial_evaluator in
plugins/nemo-optimization/src/nemo_optimization/backends/optuna/backend.py:82-88
with output_dir: Path and return type TrialEvaluator. Annotate generate_id in
plugins/nemo-optimization/src/nemo_optimization/backends/optuna/atif_metadata.py:18-18
as Callable[[], str], adding or reusing the necessary imports.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/config_overlay.py`:
- Around line 36-51: Update nest_dotted_paths to detect collisions in both
insertion orders: when assigning a leaf value, reject replacing an existing
mapping, while preserving the current error for traversing through a scalar.
Ensure inputs such as {"a": 2, "a.b": 1} and the reverse order consistently
raise KeyError instead of overwriting data.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/fabric_trial.py`:
- Around line 48-60: Remove the redundant isinstance(fabric_eval, Mapping)
guards from the _fabric_base_dir, _timeout_s, and _capture_trajectory
assignments in the surrounding initialization flow. Since fabric_eval is already
normalized to a Mapping or empty dictionary, access its values directly while
preserving the existing defaults.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/study_driver.py`:
- Around line 86-126: Update parse_numeric_study_config so each resolved
MetricSpec.name, including evaluator_name fallbacks, is unique before appending
it to metrics. Track previously resolved names and raise StudyDriverError when a
duplicate occurs, preventing duplicate entries from reaching the study
configuration.
In `@plugins/nemo-optimization/src/nemo_optimization/registry.py`:
- Around line 10-13: Update registry.py to remove the TYPE_CHECKING-only import
block and import OptimizationBackend regularly from
nemo_optimization.backends.protocol at module scope, including the other
referenced occurrence.
In `@plugins/nemo-optimization/tests/test_atif_metadata.py`:
- Around line 30-37: Extend test_build_atif_trial_tags with a case where row_id
is omitted or falsy, asserting ATIF_ROW_ID is absent from the returned tags. Add
a separate test for resolve_experiment_id using whitespace-padded input and
assert the experiment ID is stripped as expected.
In `@plugins/nemo-optimization/tests/test_fabric.py`:
- Around line 37-39: Add a test alongside test_require_fabric_agent_rejects_nat
that passes a schema-less, non-Fabric/non-NAT configuration to
require_fabric_agent_config and asserts FabricOptimizeError with a match for
“must declare schema_version”.
In `@plugins/nemo-optimization/tests/test_search_space.py`:
- Around line 115-127: Add tests in test_suggestions_by_path_maps_logical_names
covering two logical names mapped to the same path and asserting
suggestions_by_path raises the expected exception, plus a search-space case with
low >= high asserting the parser rejects it. Reuse the existing
parse_search_space and suggestions_by_path setup and preserve the current valid
mapping assertion.
🪄 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: 0ba54a52-d6d6-4dc2-9eae-26c83c0522f3
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (84)
docs/agents/index.mdxdocs/agents/optimization.mdxpackages/nemo_evaluator_sdk/pyproject.tomlpackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/__init__.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hook_loading.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks_mcp_binding.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/enums.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_defaults.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_evaluator.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/types.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/__init__.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/metrics.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_hook_loading.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_runtime.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_mcp_run_binding_hook.pypackages/nemo_evaluator_sdk/tests/metrics/test_tunable_rag_evaluator.pypackages/nemo_platform/pyproject.tomlpackages/nemo_platform_plugin/tests/test_dispatcher.pyplugins/nemo-agents/pyproject.tomlplugins/nemo-agents/src/nemo_agents_plugin/cli.pyplugins/nemo-agents/src/nemo_agents_plugin/jobs/optimize_agent.pyplugins/nemo-agents/src/nemo_agents_plugin/leaderboard/render.pyplugins/nemo-agents/src/nemo_agents_plugin/service.pyplugins/nemo-agents/src/nemo_agents_plugin/utils.pyplugins/nemo-agents/tests/unit/test_cli.pyplugins/nemo-agents/tests/unit/test_improvement_jobs.pyplugins/nemo-agents/tests/unit/test_optimize_agent_job.pyplugins/nemo-agents/tests/unit/test_optimize_skills_analyze_only.pyplugins/nemo-agents/tests/unit/test_service.pyplugins/nemo-agents/tests/unit/test_utils.pyplugins/nemo-agents/tests/unit/usage/test_usage_cli.pyplugins/nemo-optimization/README.mdplugins/nemo-optimization/examples/hermes-optimize/.gitignoreplugins/nemo-optimization/examples/hermes-optimize/README.mdplugins/nemo-optimization/examples/hermes-optimize/agent.yamlplugins/nemo-optimization/examples/hermes-optimize/analyzer.inference-api.yamlplugins/nemo-optimization/examples/hermes-optimize/dataset-phishing.jsonplugins/nemo-optimization/examples/hermes-optimize/dataset.jsonplugins/nemo-optimization/examples/hermes-optimize/optimize.yamlplugins/nemo-optimization/examples/hermes-optimize/package.yamlplugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-chatonly.yamlplugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-mcp.e2e.yamlplugins/nemo-optimization/pyproject.tomlplugins/nemo-optimization/src/nemo_optimization/__init__.pyplugins/nemo-optimization/src/nemo_optimization/agents.pyplugins/nemo-optimization/src/nemo_optimization/backends/__init__.pyplugins/nemo-optimization/src/nemo_optimization/backends/ga/__init__.pyplugins/nemo-optimization/src/nemo_optimization/backends/ga/backend.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/__init__.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/artifacts.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/atif_metadata.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/backend.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/config_overlay.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/early_stop.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/fabric_trial.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/search_space.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/selection.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/study_driver.pyplugins/nemo-optimization/src/nemo_optimization/backends/protocol.pyplugins/nemo-optimization/src/nemo_optimization/config.pyplugins/nemo-optimization/src/nemo_optimization/fabric.pyplugins/nemo-optimization/src/nemo_optimization/jobs/__init__.pyplugins/nemo-optimization/src/nemo_optimization/jobs/optimize.pyplugins/nemo-optimization/src/nemo_optimization/preflight.pyplugins/nemo-optimization/src/nemo_optimization/registry.pyplugins/nemo-optimization/src/nemo_optimization/router.pyplugins/nemo-optimization/src/nemo_optimization/schemas/__init__.pyplugins/nemo-optimization/src/nemo_optimization/schemas/optimize.pyplugins/nemo-optimization/src/nemo_optimization/tasks/__init__.pyplugins/nemo-optimization/src/nemo_optimization/tasks/optimize.pyplugins/nemo-optimization/tests/conftest.pyplugins/nemo-optimization/tests/smoke_fabric_optimize_atif.pyplugins/nemo-optimization/tests/test_atif_metadata.pyplugins/nemo-optimization/tests/test_config_overlay.pyplugins/nemo-optimization/tests/test_fabric.pyplugins/nemo-optimization/tests/test_fabric_trial.pyplugins/nemo-optimization/tests/test_optimize_job.pyplugins/nemo-optimization/tests/test_router.pyplugins/nemo-optimization/tests/test_search_space.pyplugins/nemo-optimization/tests/test_selection.pyplugins/nemo-optimization/tests/test_study_driver.pypyproject.toml
💤 Files with no reviewable changes (3)
- plugins/nemo-agents/tests/unit/test_optimize_agent_job.py
- plugins/nemo-agents/src/nemo_agents_plugin/jobs/optimize_agent.py
- plugins/nemo-agents/tests/unit/test_utils.py
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (4)
plugins/nemo-optimization/src/nemo_optimization/backends/optuna/backend.py (1)
82-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAnnotate
output_dirand the return type.
output_dirhas no annotation, and_build_trial_evaluatorhas no return type. The guidelines require concrete type hints.♻️ Proposed refactor
+from pathlib import Path + +from nemo_optimization.backends.optuna.study_driver import TrialEvaluator + def _build_trial_evaluator( payload: dict[str, Any], *, metric_names: tuple[str, ...], - output_dir, + output_dir: Path, experiment_id: str, -): +) -> TrialEvaluator:As per coding guidelines: "Prefer concrete type hints over string-based type hints".
🤖 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-optimization/src/nemo_optimization/backends/optuna/backend.py` around lines 82 - 88, Update _build_trial_evaluator by adding a concrete type annotation for output_dir and an explicit concrete return-type annotation. Follow the existing project typing conventions and avoid string-based forward references.Source: Coding guidelines
plugins/nemo-optimization/src/nemo_optimization/backends/optuna/config_overlay.py (1)
54-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLines 58-61 are dead code.
_OPTIMIZER_ONLY_TOP_LEVEL_KEYScontains"optimizer", so the loop at Line 56 already removes it.config.get("optimizer")is alwaysNoneat Line 58. If you intended to keep theoptimizerblock and strip only its search fields, remove"optimizer"from the top-level key set instead.♻️ Proposed cleanup (keeps current behavior)
def strip_optimizer_only_fields(config: dict[str, Any]) -> None: """Remove optimizer metadata from a trial config artifact (in-place).""" for key in _OPTIMIZER_ONLY_TOP_LEVEL_KEYS: config.pop(key, None) - optimizer = config.get("optimizer") - if isinstance(optimizer, dict): - optimizer.pop("search_space", None) - optimizer.pop("optimizable_params", None)🤖 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-optimization/src/nemo_optimization/backends/optuna/config_overlay.py` around lines 54 - 61, Remove the unreachable optimizer cleanup block from strip_optimizer_only_fields, since _OPTIMIZER_ONLY_TOP_LEVEL_KEYS already removes the optimizer entry before it is accessed. Preserve the current behavior of removing the entire optimizer block.plugins/nemo-agents/tests/unit/usage/test_usage_cli.py (1)
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove postponed annotations from both test modules.
The direct imports already provide concrete types. Remove
from __future__ import annotationsafter confirming the declared Python target.
plugins/nemo-agents/tests/unit/usage/test_usage_cli.py#L6-L6: remove the postponed-annotations import.plugins/nemo-optimization/tests/test_optimize_job.py#L4-L4: remove the postponed-annotations import.As per coding guidelines, “Prefer concrete type hints over string-based type hints.”
🤖 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-agents/tests/unit/usage/test_usage_cli.py` at line 6, Remove the from __future__ import annotations statement from both test modules: plugins/nemo-agents/tests/unit/usage/test_usage_cli.py at lines 6-6 and plugins/nemo-optimization/tests/test_optimize_job.py at lines 4-4. Confirm the declared Python target supports this removal and leave the existing concrete type hints unchanged.Source: Coding guidelines
plugins/nemo-optimization/tests/test_optimize_job.py (1)
35-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRun the task module through
uv run.Line 37 locks the platform task to
python -m. UpdateOptimizeJob.compileand this assertion to invoke the module throughuv run. Confirm that the task image includesuv.As per coding guidelines, “Run Python scripts and tools through uv.”
🤖 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-optimization/tests/test_optimize_job.py` around lines 35 - 37, Update OptimizeJob.compile to invoke the optimize task module through uv run instead of python -m, and update the corresponding command assertion in test_optimize_job.py to expect the new invocation. Verify the task image used by OptimizeJob includes uv so the generated command is executable.Source: Coding guidelines
🤖 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_evaluator_sdk/tests/agent_eval/test_mcp_run_binding_hook.py`:
- Around line 242-246: Remove the `(pkg / "__init__.py").write_text(...)` setup
from `test_mcp_run_binding_path_based_ref`; the temporary `agent_pkg` directory
must rely on an implicit namespace package while leaving the rest of the test
fixture unchanged.
In `@plugins/nemo-agents/tests/unit/usage/test_usage_cli.py`:
- Around line 204-220: Update
test_usage_show_unreadable_result_json_exits_cleanly to mock the result-file
read operation so it raises PermissionError instead of relying on chmod(0o000).
Remove the permission-changing setup and cleanup, while preserving the
assertions for exit code 1, no traceback, and the “cannot read file” message.
- Around line 105-112: Update test_usage_with_no_args_prints_help to assert a
non-zero result.exit_code directly, replacing the current acceptance of both 0
and 2; keep the help-output assertion unchanged.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/artifacts.py`:
- Around line 111-114: Handle studies with no completed trials consistently:
update _pareto_trial_numbers to return an empty set instead of accessing
best_trial or best_trials when no completed result exists, and update
run_numeric_study to avoid raising before artifact writers in the same state.
Preserve existing Pareto selection for studies with completed trials, and add a
regression test covering all trials failing or being pruned.
In `@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/backend.py`:
- Around line 44-47: Update the exception handling around payload access in the
optimizer configuration flow to handle a missing payload["optimizer"] key
explicitly, raising StudyDriverError with contextual information that identifies
the missing optimizer configuration instead of the bare quoted key. Preserve the
existing parse_numeric_study_config handling for StudyDriverError and other
parsing failures.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/fabric_trial.py`:
- Line 47: Update the trial evaluation flow around build_agent_eval_tasks,
_tasks, and evaluate() so evaluator tasks are rebuilt from each trial’s
suggestion-resolved payload rather than the base payload. Pass those per-trial
tasks to run_sync, ensuring eval.evaluators search-space settings affect
scoring, and add a regression test covering an evaluator setting.
- Around line 219-229: Update the model URL validation in the surrounding
model-construction logic before new_inference_client creates AsyncOpenAI: reject
credentialed judge configurations using an HTTP URL, while permitting HTTPS URLs
and non-credentialed HTTP URLs as appropriate. Use the existing secret_ref and
parsed URL values, and raise StudyDriverError with the model name when the
invalid combination is detected.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/search_space.py`:
- Around line 96-105: Update the range-bound validation in the search-space
parsing logic around low and high so both values are numeric before comparing
them or passing them to suggest_float. Raise SearchSpaceError for non-numeric
bounds, while preserving the existing requirement that both bounds are present
and low is less than high.
In `@plugins/nemo-optimization/tests/test_fabric_trial.py`:
- Around line 184-199: Update the test invocation in
FabricTrialEvaluator.evaluate to pass the logical suggestion name temperature
instead of the Fabric path models.default.temperature, and keep the expected
configuration assertion unchanged. In evaluate, resolve each logical suggestion
through suggestions_by_path before applying it so run_numeric_study inputs map
to the correct dotted configuration path.
---
Nitpick comments:
In `@plugins/nemo-agents/tests/unit/usage/test_usage_cli.py`:
- Line 6: Remove the from __future__ import annotations statement from both test
modules: plugins/nemo-agents/tests/unit/usage/test_usage_cli.py at lines 6-6 and
plugins/nemo-optimization/tests/test_optimize_job.py at lines 4-4. Confirm the
declared Python target supports this removal and leave the existing concrete
type hints unchanged.
In `@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/backend.py`:
- Around line 82-88: Update _build_trial_evaluator by adding a concrete type
annotation for output_dir and an explicit concrete return-type annotation.
Follow the existing project typing conventions and avoid string-based forward
references.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/config_overlay.py`:
- Around line 54-61: Remove the unreachable optimizer cleanup block from
strip_optimizer_only_fields, since _OPTIMIZER_ONLY_TOP_LEVEL_KEYS already
removes the optimizer entry before it is accessed. Preserve the current behavior
of removing the entire optimizer block.
In `@plugins/nemo-optimization/tests/test_optimize_job.py`:
- Around line 35-37: Update OptimizeJob.compile to invoke the optimize task
module through uv run instead of python -m, and update the corresponding command
assertion in test_optimize_job.py to expect the new invocation. Verify the task
image used by OptimizeJob includes uv so the generated command is executable.
🪄 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: f6064e55-1f90-446f-a738-ac0abcbb398f
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (84)
docs/agents/index.mdxdocs/agents/optimization.mdxpackages/nemo_evaluator_sdk/pyproject.tomlpackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/__init__.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hook_loading.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks_mcp_binding.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/enums.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_defaults.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_evaluator.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/types.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/__init__.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/metrics.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_hook_loading.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_runtime.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_mcp_run_binding_hook.pypackages/nemo_evaluator_sdk/tests/metrics/test_tunable_rag_evaluator.pypackages/nemo_platform/pyproject.tomlpackages/nemo_platform_plugin/tests/test_dispatcher.pyplugins/nemo-agents/pyproject.tomlplugins/nemo-agents/src/nemo_agents_plugin/cli.pyplugins/nemo-agents/src/nemo_agents_plugin/jobs/optimize_agent.pyplugins/nemo-agents/src/nemo_agents_plugin/leaderboard/render.pyplugins/nemo-agents/src/nemo_agents_plugin/service.pyplugins/nemo-agents/src/nemo_agents_plugin/utils.pyplugins/nemo-agents/tests/unit/test_cli.pyplugins/nemo-agents/tests/unit/test_improvement_jobs.pyplugins/nemo-agents/tests/unit/test_optimize_agent_job.pyplugins/nemo-agents/tests/unit/test_optimize_skills_analyze_only.pyplugins/nemo-agents/tests/unit/test_service.pyplugins/nemo-agents/tests/unit/test_utils.pyplugins/nemo-agents/tests/unit/usage/test_usage_cli.pyplugins/nemo-optimization/README.mdplugins/nemo-optimization/examples/hermes-optimize/.gitignoreplugins/nemo-optimization/examples/hermes-optimize/README.mdplugins/nemo-optimization/examples/hermes-optimize/agent.yamlplugins/nemo-optimization/examples/hermes-optimize/analyzer.inference-api.yamlplugins/nemo-optimization/examples/hermes-optimize/dataset-phishing.jsonplugins/nemo-optimization/examples/hermes-optimize/dataset.jsonplugins/nemo-optimization/examples/hermes-optimize/optimize.yamlplugins/nemo-optimization/examples/hermes-optimize/package.yamlplugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-chatonly.yamlplugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-mcp.e2e.yamlplugins/nemo-optimization/pyproject.tomlplugins/nemo-optimization/src/nemo_optimization/__init__.pyplugins/nemo-optimization/src/nemo_optimization/agents.pyplugins/nemo-optimization/src/nemo_optimization/backends/__init__.pyplugins/nemo-optimization/src/nemo_optimization/backends/ga/__init__.pyplugins/nemo-optimization/src/nemo_optimization/backends/ga/backend.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/__init__.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/artifacts.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/atif_metadata.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/backend.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/config_overlay.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/early_stop.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/fabric_trial.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/search_space.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/selection.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/study_driver.pyplugins/nemo-optimization/src/nemo_optimization/backends/protocol.pyplugins/nemo-optimization/src/nemo_optimization/config.pyplugins/nemo-optimization/src/nemo_optimization/fabric.pyplugins/nemo-optimization/src/nemo_optimization/jobs/__init__.pyplugins/nemo-optimization/src/nemo_optimization/jobs/optimize.pyplugins/nemo-optimization/src/nemo_optimization/preflight.pyplugins/nemo-optimization/src/nemo_optimization/registry.pyplugins/nemo-optimization/src/nemo_optimization/router.pyplugins/nemo-optimization/src/nemo_optimization/schemas/__init__.pyplugins/nemo-optimization/src/nemo_optimization/schemas/optimize.pyplugins/nemo-optimization/src/nemo_optimization/tasks/__init__.pyplugins/nemo-optimization/src/nemo_optimization/tasks/optimize.pyplugins/nemo-optimization/tests/conftest.pyplugins/nemo-optimization/tests/smoke_fabric_optimize_atif.pyplugins/nemo-optimization/tests/test_atif_metadata.pyplugins/nemo-optimization/tests/test_config_overlay.pyplugins/nemo-optimization/tests/test_fabric.pyplugins/nemo-optimization/tests/test_fabric_trial.pyplugins/nemo-optimization/tests/test_optimize_job.pyplugins/nemo-optimization/tests/test_router.pyplugins/nemo-optimization/tests/test_search_space.pyplugins/nemo-optimization/tests/test_selection.pyplugins/nemo-optimization/tests/test_study_driver.pypyproject.toml
💤 Files with no reviewable changes (3)
- plugins/nemo-agents/tests/unit/test_optimize_agent_job.py
- plugins/nemo-agents/src/nemo_agents_plugin/jobs/optimize_agent.py
- plugins/nemo-agents/tests/unit/test_utils.py
🚧 Files skipped from review as they are similar to previous changes (63)
- pyproject.toml
- plugins/nemo-optimization/examples/hermes-optimize/analyzer.inference-api.yaml
- plugins/nemo-optimization/examples/hermes-optimize/dataset.json
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/init.py
- plugins/nemo-optimization/src/nemo_optimization/tasks/init.py
- plugins/nemo-optimization/src/nemo_optimization/backends/init.py
- plugins/nemo-agents/tests/unit/test_service.py
- packages/nemo_platform/pyproject.toml
- plugins/nemo-optimization/src/nemo_optimization/init.py
- plugins/nemo-agents/src/nemo_agents_plugin/service.py
- packages/nemo_evaluator_sdk/pyproject.toml
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/init.py
- plugins/nemo-optimization/src/nemo_optimization/jobs/init.py
- plugins/nemo-optimization/tests/test_search_space.py
- plugins/nemo-optimization/examples/hermes-optimize/dataset-phishing.json
- plugins/nemo-optimization/tests/test_config_overlay.py
- plugins/nemo-optimization/src/nemo_optimization/backends/protocol.py
- plugins/nemo-optimization/examples/hermes-optimize/agent.yaml
- plugins/nemo-optimization/examples/hermes-optimize/.gitignore
- plugins/nemo-optimization/src/nemo_optimization/schemas/optimize.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/enums.py
- plugins/nemo-agents/tests/unit/test_improvement_jobs.py
- plugins/nemo-optimization/src/nemo_optimization/registry.py
- plugins/nemo-optimization/tests/conftest.py
- plugins/nemo-optimization/examples/hermes-optimize/package.yaml
- plugins/nemo-optimization/tests/test_selection.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/types.py
- plugins/nemo-optimization/src/nemo_optimization/tasks/optimize.py
- plugins/nemo-optimization/src/nemo_optimization/schemas/init.py
- plugins/nemo-agents/src/nemo_agents_plugin/utils.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_runtime.py
- plugins/nemo-optimization/src/nemo_optimization/backends/ga/init.py
- packages/nemo_platform_plugin/tests/test_dispatcher.py
- plugins/nemo-agents/pyproject.toml
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/atif_metadata.py
- plugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-mcp.e2e.yaml
- plugins/nemo-optimization/src/nemo_optimization/jobs/optimize.py
- plugins/nemo-optimization/README.md
- plugins/nemo-optimization/tests/test_atif_metadata.py
- docs/agents/optimization.mdx
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hook_loading.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/early_stop.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/selection.py
- plugins/nemo-optimization/src/nemo_optimization/backends/ga/backend.py
- plugins/nemo-optimization/tests/test_router.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks.py
- plugins/nemo-agents/src/nemo_agents_plugin/leaderboard/render.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_defaults.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/metrics.py
- plugins/nemo-optimization/src/nemo_optimization/preflight.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/init.py
- plugins/nemo-optimization/src/nemo_optimization/router.py
- plugins/nemo-agents/src/nemo_agents_plugin/cli.py
- plugins/nemo-optimization/examples/hermes-optimize/README.md
- plugins/nemo-optimization/tests/test_study_driver.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_hook_loading.py
- plugins/nemo-optimization/src/nemo_optimization/agents.py
- plugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-chatonly.yaml
- plugins/nemo-optimization/tests/test_fabric.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks_mcp_binding.py
- plugins/nemo-optimization/pyproject.toml
- plugins/nemo-optimization/src/nemo_optimization/fabric.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_evaluator.py
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 9
🧹 Nitpick comments (4)
plugins/nemo-optimization/src/nemo_optimization/backends/optuna/backend.py (1)
82-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAnnotate
output_dirand the return type.
output_dirhas no annotation, and_build_trial_evaluatorhas no return type. The guidelines require concrete type hints.♻️ Proposed refactor
+from pathlib import Path + +from nemo_optimization.backends.optuna.study_driver import TrialEvaluator + def _build_trial_evaluator( payload: dict[str, Any], *, metric_names: tuple[str, ...], - output_dir, + output_dir: Path, experiment_id: str, -): +) -> TrialEvaluator:As per coding guidelines: "Prefer concrete type hints over string-based type hints".
🤖 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-optimization/src/nemo_optimization/backends/optuna/backend.py` around lines 82 - 88, Update _build_trial_evaluator by adding a concrete type annotation for output_dir and an explicit concrete return-type annotation. Follow the existing project typing conventions and avoid string-based forward references.Source: Coding guidelines
plugins/nemo-optimization/src/nemo_optimization/backends/optuna/config_overlay.py (1)
54-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLines 58-61 are dead code.
_OPTIMIZER_ONLY_TOP_LEVEL_KEYScontains"optimizer", so the loop at Line 56 already removes it.config.get("optimizer")is alwaysNoneat Line 58. If you intended to keep theoptimizerblock and strip only its search fields, remove"optimizer"from the top-level key set instead.♻️ Proposed cleanup (keeps current behavior)
def strip_optimizer_only_fields(config: dict[str, Any]) -> None: """Remove optimizer metadata from a trial config artifact (in-place).""" for key in _OPTIMIZER_ONLY_TOP_LEVEL_KEYS: config.pop(key, None) - optimizer = config.get("optimizer") - if isinstance(optimizer, dict): - optimizer.pop("search_space", None) - optimizer.pop("optimizable_params", None)🤖 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-optimization/src/nemo_optimization/backends/optuna/config_overlay.py` around lines 54 - 61, Remove the unreachable optimizer cleanup block from strip_optimizer_only_fields, since _OPTIMIZER_ONLY_TOP_LEVEL_KEYS already removes the optimizer entry before it is accessed. Preserve the current behavior of removing the entire optimizer block.plugins/nemo-agents/tests/unit/usage/test_usage_cli.py (1)
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove postponed annotations from both test modules.
The direct imports already provide concrete types. Remove
from __future__ import annotationsafter confirming the declared Python target.
plugins/nemo-agents/tests/unit/usage/test_usage_cli.py#L6-L6: remove the postponed-annotations import.plugins/nemo-optimization/tests/test_optimize_job.py#L4-L4: remove the postponed-annotations import.As per coding guidelines, “Prefer concrete type hints over string-based type hints.”
🤖 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-agents/tests/unit/usage/test_usage_cli.py` at line 6, Remove the from __future__ import annotations statement from both test modules: plugins/nemo-agents/tests/unit/usage/test_usage_cli.py at lines 6-6 and plugins/nemo-optimization/tests/test_optimize_job.py at lines 4-4. Confirm the declared Python target supports this removal and leave the existing concrete type hints unchanged.Source: Coding guidelines
plugins/nemo-optimization/tests/test_optimize_job.py (1)
35-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRun the task module through
uv run.Line 37 locks the platform task to
python -m. UpdateOptimizeJob.compileand this assertion to invoke the module throughuv run. Confirm that the task image includesuv.As per coding guidelines, “Run Python scripts and tools through uv.”
🤖 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-optimization/tests/test_optimize_job.py` around lines 35 - 37, Update OptimizeJob.compile to invoke the optimize task module through uv run instead of python -m, and update the corresponding command assertion in test_optimize_job.py to expect the new invocation. Verify the task image used by OptimizeJob includes uv so the generated command is executable.Source: Coding guidelines
🤖 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_evaluator_sdk/tests/agent_eval/test_mcp_run_binding_hook.py`:
- Around line 242-246: Remove the `(pkg / "__init__.py").write_text(...)` setup
from `test_mcp_run_binding_path_based_ref`; the temporary `agent_pkg` directory
must rely on an implicit namespace package while leaving the rest of the test
fixture unchanged.
In `@plugins/nemo-agents/tests/unit/usage/test_usage_cli.py`:
- Around line 204-220: Update
test_usage_show_unreadable_result_json_exits_cleanly to mock the result-file
read operation so it raises PermissionError instead of relying on chmod(0o000).
Remove the permission-changing setup and cleanup, while preserving the
assertions for exit code 1, no traceback, and the “cannot read file” message.
- Around line 105-112: Update test_usage_with_no_args_prints_help to assert a
non-zero result.exit_code directly, replacing the current acceptance of both 0
and 2; keep the help-output assertion unchanged.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/artifacts.py`:
- Around line 111-114: Handle studies with no completed trials consistently:
update _pareto_trial_numbers to return an empty set instead of accessing
best_trial or best_trials when no completed result exists, and update
run_numeric_study to avoid raising before artifact writers in the same state.
Preserve existing Pareto selection for studies with completed trials, and add a
regression test covering all trials failing or being pruned.
In `@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/backend.py`:
- Around line 44-47: Update the exception handling around payload access in the
optimizer configuration flow to handle a missing payload["optimizer"] key
explicitly, raising StudyDriverError with contextual information that identifies
the missing optimizer configuration instead of the bare quoted key. Preserve the
existing parse_numeric_study_config handling for StudyDriverError and other
parsing failures.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/fabric_trial.py`:
- Line 47: Update the trial evaluation flow around build_agent_eval_tasks,
_tasks, and evaluate() so evaluator tasks are rebuilt from each trial’s
suggestion-resolved payload rather than the base payload. Pass those per-trial
tasks to run_sync, ensuring eval.evaluators search-space settings affect
scoring, and add a regression test covering an evaluator setting.
- Around line 219-229: Update the model URL validation in the surrounding
model-construction logic before new_inference_client creates AsyncOpenAI: reject
credentialed judge configurations using an HTTP URL, while permitting HTTPS URLs
and non-credentialed HTTP URLs as appropriate. Use the existing secret_ref and
parsed URL values, and raise StudyDriverError with the model name when the
invalid combination is detected.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/search_space.py`:
- Around line 96-105: Update the range-bound validation in the search-space
parsing logic around low and high so both values are numeric before comparing
them or passing them to suggest_float. Raise SearchSpaceError for non-numeric
bounds, while preserving the existing requirement that both bounds are present
and low is less than high.
In `@plugins/nemo-optimization/tests/test_fabric_trial.py`:
- Around line 184-199: Update the test invocation in
FabricTrialEvaluator.evaluate to pass the logical suggestion name temperature
instead of the Fabric path models.default.temperature, and keep the expected
configuration assertion unchanged. In evaluate, resolve each logical suggestion
through suggestions_by_path before applying it so run_numeric_study inputs map
to the correct dotted configuration path.
---
Nitpick comments:
In `@plugins/nemo-agents/tests/unit/usage/test_usage_cli.py`:
- Line 6: Remove the from __future__ import annotations statement from both test
modules: plugins/nemo-agents/tests/unit/usage/test_usage_cli.py at lines 6-6 and
plugins/nemo-optimization/tests/test_optimize_job.py at lines 4-4. Confirm the
declared Python target supports this removal and leave the existing concrete
type hints unchanged.
In `@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/backend.py`:
- Around line 82-88: Update _build_trial_evaluator by adding a concrete type
annotation for output_dir and an explicit concrete return-type annotation.
Follow the existing project typing conventions and avoid string-based forward
references.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/config_overlay.py`:
- Around line 54-61: Remove the unreachable optimizer cleanup block from
strip_optimizer_only_fields, since _OPTIMIZER_ONLY_TOP_LEVEL_KEYS already
removes the optimizer entry before it is accessed. Preserve the current behavior
of removing the entire optimizer block.
In `@plugins/nemo-optimization/tests/test_optimize_job.py`:
- Around line 35-37: Update OptimizeJob.compile to invoke the optimize task
module through uv run instead of python -m, and update the corresponding command
assertion in test_optimize_job.py to expect the new invocation. Verify the task
image used by OptimizeJob includes uv so the generated command is executable.
🪄 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: f6064e55-1f90-446f-a738-ac0abcbb398f
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (84)
docs/agents/index.mdxdocs/agents/optimization.mdxpackages/nemo_evaluator_sdk/pyproject.tomlpackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/__init__.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hook_loading.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks_mcp_binding.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/enums.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_defaults.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_evaluator.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/types.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/__init__.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/metrics.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_hook_loading.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_runtime.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_mcp_run_binding_hook.pypackages/nemo_evaluator_sdk/tests/metrics/test_tunable_rag_evaluator.pypackages/nemo_platform/pyproject.tomlpackages/nemo_platform_plugin/tests/test_dispatcher.pyplugins/nemo-agents/pyproject.tomlplugins/nemo-agents/src/nemo_agents_plugin/cli.pyplugins/nemo-agents/src/nemo_agents_plugin/jobs/optimize_agent.pyplugins/nemo-agents/src/nemo_agents_plugin/leaderboard/render.pyplugins/nemo-agents/src/nemo_agents_plugin/service.pyplugins/nemo-agents/src/nemo_agents_plugin/utils.pyplugins/nemo-agents/tests/unit/test_cli.pyplugins/nemo-agents/tests/unit/test_improvement_jobs.pyplugins/nemo-agents/tests/unit/test_optimize_agent_job.pyplugins/nemo-agents/tests/unit/test_optimize_skills_analyze_only.pyplugins/nemo-agents/tests/unit/test_service.pyplugins/nemo-agents/tests/unit/test_utils.pyplugins/nemo-agents/tests/unit/usage/test_usage_cli.pyplugins/nemo-optimization/README.mdplugins/nemo-optimization/examples/hermes-optimize/.gitignoreplugins/nemo-optimization/examples/hermes-optimize/README.mdplugins/nemo-optimization/examples/hermes-optimize/agent.yamlplugins/nemo-optimization/examples/hermes-optimize/analyzer.inference-api.yamlplugins/nemo-optimization/examples/hermes-optimize/dataset-phishing.jsonplugins/nemo-optimization/examples/hermes-optimize/dataset.jsonplugins/nemo-optimization/examples/hermes-optimize/optimize.yamlplugins/nemo-optimization/examples/hermes-optimize/package.yamlplugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-chatonly.yamlplugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-mcp.e2e.yamlplugins/nemo-optimization/pyproject.tomlplugins/nemo-optimization/src/nemo_optimization/__init__.pyplugins/nemo-optimization/src/nemo_optimization/agents.pyplugins/nemo-optimization/src/nemo_optimization/backends/__init__.pyplugins/nemo-optimization/src/nemo_optimization/backends/ga/__init__.pyplugins/nemo-optimization/src/nemo_optimization/backends/ga/backend.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/__init__.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/artifacts.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/atif_metadata.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/backend.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/config_overlay.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/early_stop.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/fabric_trial.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/search_space.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/selection.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/study_driver.pyplugins/nemo-optimization/src/nemo_optimization/backends/protocol.pyplugins/nemo-optimization/src/nemo_optimization/config.pyplugins/nemo-optimization/src/nemo_optimization/fabric.pyplugins/nemo-optimization/src/nemo_optimization/jobs/__init__.pyplugins/nemo-optimization/src/nemo_optimization/jobs/optimize.pyplugins/nemo-optimization/src/nemo_optimization/preflight.pyplugins/nemo-optimization/src/nemo_optimization/registry.pyplugins/nemo-optimization/src/nemo_optimization/router.pyplugins/nemo-optimization/src/nemo_optimization/schemas/__init__.pyplugins/nemo-optimization/src/nemo_optimization/schemas/optimize.pyplugins/nemo-optimization/src/nemo_optimization/tasks/__init__.pyplugins/nemo-optimization/src/nemo_optimization/tasks/optimize.pyplugins/nemo-optimization/tests/conftest.pyplugins/nemo-optimization/tests/smoke_fabric_optimize_atif.pyplugins/nemo-optimization/tests/test_atif_metadata.pyplugins/nemo-optimization/tests/test_config_overlay.pyplugins/nemo-optimization/tests/test_fabric.pyplugins/nemo-optimization/tests/test_fabric_trial.pyplugins/nemo-optimization/tests/test_optimize_job.pyplugins/nemo-optimization/tests/test_router.pyplugins/nemo-optimization/tests/test_search_space.pyplugins/nemo-optimization/tests/test_selection.pyplugins/nemo-optimization/tests/test_study_driver.pypyproject.toml
💤 Files with no reviewable changes (3)
- plugins/nemo-agents/tests/unit/test_optimize_agent_job.py
- plugins/nemo-agents/src/nemo_agents_plugin/jobs/optimize_agent.py
- plugins/nemo-agents/tests/unit/test_utils.py
🚧 Files skipped from review as they are similar to previous changes (63)
- pyproject.toml
- plugins/nemo-optimization/examples/hermes-optimize/analyzer.inference-api.yaml
- plugins/nemo-optimization/examples/hermes-optimize/dataset.json
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/init.py
- plugins/nemo-optimization/src/nemo_optimization/tasks/init.py
- plugins/nemo-optimization/src/nemo_optimization/backends/init.py
- plugins/nemo-agents/tests/unit/test_service.py
- packages/nemo_platform/pyproject.toml
- plugins/nemo-optimization/src/nemo_optimization/init.py
- plugins/nemo-agents/src/nemo_agents_plugin/service.py
- packages/nemo_evaluator_sdk/pyproject.toml
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/init.py
- plugins/nemo-optimization/src/nemo_optimization/jobs/init.py
- plugins/nemo-optimization/tests/test_search_space.py
- plugins/nemo-optimization/examples/hermes-optimize/dataset-phishing.json
- plugins/nemo-optimization/tests/test_config_overlay.py
- plugins/nemo-optimization/src/nemo_optimization/backends/protocol.py
- plugins/nemo-optimization/examples/hermes-optimize/agent.yaml
- plugins/nemo-optimization/examples/hermes-optimize/.gitignore
- plugins/nemo-optimization/src/nemo_optimization/schemas/optimize.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/enums.py
- plugins/nemo-agents/tests/unit/test_improvement_jobs.py
- plugins/nemo-optimization/src/nemo_optimization/registry.py
- plugins/nemo-optimization/tests/conftest.py
- plugins/nemo-optimization/examples/hermes-optimize/package.yaml
- plugins/nemo-optimization/tests/test_selection.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/types.py
- plugins/nemo-optimization/src/nemo_optimization/tasks/optimize.py
- plugins/nemo-optimization/src/nemo_optimization/schemas/init.py
- plugins/nemo-agents/src/nemo_agents_plugin/utils.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_runtime.py
- plugins/nemo-optimization/src/nemo_optimization/backends/ga/init.py
- packages/nemo_platform_plugin/tests/test_dispatcher.py
- plugins/nemo-agents/pyproject.toml
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/atif_metadata.py
- plugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-mcp.e2e.yaml
- plugins/nemo-optimization/src/nemo_optimization/jobs/optimize.py
- plugins/nemo-optimization/README.md
- plugins/nemo-optimization/tests/test_atif_metadata.py
- docs/agents/optimization.mdx
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hook_loading.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/early_stop.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/selection.py
- plugins/nemo-optimization/src/nemo_optimization/backends/ga/backend.py
- plugins/nemo-optimization/tests/test_router.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks.py
- plugins/nemo-agents/src/nemo_agents_plugin/leaderboard/render.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_defaults.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/metrics.py
- plugins/nemo-optimization/src/nemo_optimization/preflight.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/init.py
- plugins/nemo-optimization/src/nemo_optimization/router.py
- plugins/nemo-agents/src/nemo_agents_plugin/cli.py
- plugins/nemo-optimization/examples/hermes-optimize/README.md
- plugins/nemo-optimization/tests/test_study_driver.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_hook_loading.py
- plugins/nemo-optimization/src/nemo_optimization/agents.py
- plugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-chatonly.yaml
- plugins/nemo-optimization/tests/test_fabric.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks_mcp_binding.py
- plugins/nemo-optimization/pyproject.toml
- plugins/nemo-optimization/src/nemo_optimization/fabric.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_evaluator.py
🛑 Comments failed to post (2)
plugins/nemo-agents/tests/unit/usage/test_usage_cli.py (2)
105-112: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the documented exit behavior.
Line 106 says this command exits non-zero. Line 110 accepts zero. Assert a non-zero exit code so a success exit does not mask a CLI contract regression.
🤖 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-agents/tests/unit/usage/test_usage_cli.py` around lines 105 - 112, Update test_usage_with_no_args_prints_help to assert a non-zero result.exit_code directly, replacing the current acceptance of both 0 and 2; keep the help-output assertion unchanged.
204-220: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Make the read-error test deterministic.
chmod(0o000)does not deny reads to privileged processes and has platform-dependent behavior. Mock the result-file read path to raisePermissionErrorso this test always verifies the intended error handling.🤖 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-agents/tests/unit/usage/test_usage_cli.py` around lines 204 - 220, Update test_usage_show_unreadable_result_json_exits_cleanly to mock the result-file read operation so it raises PermissionError instead of relying on chmod(0o000). Remove the permission-changing setup and cleanup, while preserving the assertions for exit code 1, no traceback, and the “cannot read file” message.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/gpu.py`:
- Around line 206-214: Update the reservation-loading logic around
Configuration.get_service_config and NemoPlatformConfig so optional import
failures are handled separately, while ValueError and other configuration errors
from get_reserved_gpu_ids() propagate after appropriate logging. Do not fall
back to detect_gpu_device_ids() when reservation configuration is invalid; only
use that fallback when the optional configuration module is unavailable or no
reservation is configured.
🪄 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: b5d55fea-361e-4883-aa56-2356fd789f4a
📒 Files selected for processing (1)
plugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/gpu.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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-deployments/src/nemo_deployments_plugin/backends/docker/backend.py`:
- Around line 482-488: The Docker run configuration around the network and
extra_hosts setup must make host.docker.internal reachable from netns sidecars,
since Docker rejects extra_hosts with network="container:<primary>". Update the
sidecar/primary networking design to provide a shared reachable platform-host
address while preserving the existing healthcheck behavior, and add a Linux
Docker Engine integration test covering the auth-proxy sidecar request path.
In `@plugins/nemo-optimization/examples/hermes-optimize/README.md`:
- Around line 59-62: Add Python SDK counterparts in the README for both the
chat-only and bound-MCP optimization workflows alongside their existing CLI
commands, organizing each pair in the repository’s supported tab-set format.
Ensure the SDK examples are tested and remain functionally equivalent to the
corresponding CLI examples; alternatively, move the paired examples to a
documentation page that supports tab sets.
- Around line 40-43: Update the setup instructions in the Hermes optimization
README to sync/install nemo-agents-plugin, then install the matching Fabric
wheel into .venv using an explicit wheel path instead of the placeholder
command. Retain the uv run --no-sync usage for subsequent commands so the
installed Fabric version is preserved.
🪄 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: b3a207d3-d712-4145-b524-861da88cb883
📒 Files selected for processing (4)
plugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/backend.pyplugins/nemo-optimization/examples/hermes-optimize/README.mdservices/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/compiler.pyservices/core/models/tests/unit/controllers/backends/deployments_plugin/test_compiler.py
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugins/nemo-optimization/tests/test_study_driver.py (1)
175-175: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse concrete mapping type hints.
Replace bare
dictannotations with parameterized mapping types that matchTrialEvaluator. Verify withuv run --frozen ty check plugins/nemo-optimization/tests/test_study_driver.py.As per coding guidelines, use concrete type hints and
uv run --frozen ty checkfor Python type checking.🤖 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-optimization/tests/test_study_driver.py` at line 175, Update the evaluate method signature in the TrialEvaluator implementation to replace bare dict annotations for suggestions and trial_overlay with concrete parameterized mapping types matching TrialEvaluator. Preserve the existing return annotation, then verify the file with uv run --frozen ty check plugins/nemo-optimization/tests/test_study_driver.py.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@plugins/nemo-optimization/tests/test_study_driver.py`:
- Line 175: Update the evaluate method signature in the TrialEvaluator
implementation to replace bare dict annotations for suggestions and
trial_overlay with concrete parameterized mapping types matching TrialEvaluator.
Preserve the existing return annotation, then verify the file with uv run
--frozen ty check plugins/nemo-optimization/tests/test_study_driver.py.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c0113f00-7977-4f24-be9b-92044eb05f02
📒 Files selected for processing (11)
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks_mcp_binding.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_mcp_run_binding_hook.pyplugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/gpu.pyplugins/nemo-deployments/tests/unit/backends/docker/test_gpu.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/artifacts.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/fabric_trial.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/study_driver.pyplugins/nemo-optimization/src/nemo_optimization/fabric.pyplugins/nemo-optimization/tests/test_fabric.pyplugins/nemo-optimization/tests/test_fabric_trial.pyplugins/nemo-optimization/tests/test_study_driver.py
🚧 Files skipped from review as they are similar to previous changes (6)
- plugins/nemo-optimization/tests/test_fabric.py
- plugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/gpu.py
- plugins/nemo-optimization/src/nemo_optimization/fabric.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/artifacts.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/fabric_trial.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks_mcp_binding.py
soluwalana
force-pushed
the
aalgo-277/solu
branch
from
August 5, 2026 00:31
4f27459 to
c75bd60
Compare
anubhutivyas
reviewed
Aug 5, 2026
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
plugins/nemo-optimization/src/nemo_optimization/backends/optuna/artifacts.py (1)
62-64: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winBuild Pareto indexes in the filtered value index space.
_trial_valuesskips failed and pruned trials.pareto_indexesenumerates unfilteredstudy.trials. A skipped trial shifts later indexes and marks the wrong points as Pareto-optimal.Build values and Pareto indexes in one filtered pass. Add a regression test with a failed or pruned trial before a completed Pareto trial.
Proposed fix
- values = _trial_values(study.trials, len(metric_names)) pareto_numbers = _pareto_trial_numbers(study) - pareto_indexes = [index for index, trial in enumerate(study.trials) if trial.number in pareto_numbers] + values, pareto_indexes = _plot_points( + study.trials, len(metric_names), pareto_numbers + )🤖 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-optimization/src/nemo_optimization/backends/optuna/artifacts.py` around lines 62 - 64, Update the logic around _trial_values and pareto_indexes to filter failed and pruned trials once, then derive both metric values and Pareto indexes from that same filtered trial sequence. Preserve the existing Pareto membership check using trial.number, and add a regression test covering a failed or pruned trial before a completed Pareto trial.
🧹 Nitpick comments (1)
plugins/nemo-optimization/examples/hermes-optimize/README.md (1)
203-212: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a Next Steps section.
Add
## Next Stepswith links todocs/agents/optimization.mdxand the Fabric MCP workflow.🤖 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-optimization/examples/hermes-optimize/README.md` around lines 203 - 212, Add a “Next Steps” section to the README after the existing Notes, linking to docs/agents/optimization.mdx and the Fabric MCP workflow documentation.Source: Coding guidelines
🤖 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/index.mdx`:
- Around line 57-63: Remove the Optimize row from the “Legacy NAT-only commands”
table in the agents documentation, since `nemo agents optimize run` belongs to
the Fabric workflow. Add or update the surrounding documentation to direct users
to the Fabric optimization workflow, while keeping the NAT evaluation entry
unchanged.
In `@plugins/nemo-agents/tests/unit/usage/test_usage_cli.py`:
- Around line 210-216: Update the test around the usage CLI invocation to mock
the parser’s file-read operation so it raises PermissionError, rather than
relying on chmod(0o000) for bad. Remove the permission-bit manipulation and
retain the assertion coverage for handling the read failure through
runner.invoke.
---
Duplicate comments:
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/artifacts.py`:
- Around line 62-64: Update the logic around _trial_values and pareto_indexes to
filter failed and pruned trials once, then derive both metric values and Pareto
indexes from that same filtered trial sequence. Preserve the existing Pareto
membership check using trial.number, and add a regression test covering a failed
or pruned trial before a completed Pareto trial.
---
Nitpick comments:
In `@plugins/nemo-optimization/examples/hermes-optimize/README.md`:
- Around line 203-212: Add a “Next Steps” section to the README after the
existing Notes, linking to docs/agents/optimization.mdx and the Fabric MCP
workflow documentation.
🪄 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: d67004fd-d1d8-4754-ab90-7fd8a89cde5d
⛔ Files ignored due to path filters (12)
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/hook_loading.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/hooks.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/hooks_mcp_binding.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/runtime.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/enums.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/metrics/tunable_rag_defaults.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/metrics/tunable_rag_evaluator.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/metrics/types.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/values/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/values/metrics.pyis excluded by!sdk/**uv.lockis excluded by!**/*.lock
📒 Files selected for processing (93)
docs/agents/index.mdxdocs/agents/optimization.mdxpackages/nemo_evaluator_sdk/pyproject.tomlpackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/__init__.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hook_loading.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks_mcp_binding.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/enums.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_defaults.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_evaluator.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/types.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/__init__.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/metrics.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_hook_loading.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_runtime.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_mcp_run_binding_hook.pypackages/nemo_evaluator_sdk/tests/metrics/test_tunable_rag_evaluator.pypackages/nemo_platform/pyproject.tomlpackages/nemo_platform_plugin/tests/test_dispatcher.pyplugins/nemo-agents/openapi/openapi.yamlplugins/nemo-agents/pyproject.tomlplugins/nemo-agents/src/nemo_agents_plugin/cli.pyplugins/nemo-agents/src/nemo_agents_plugin/jobs/optimize_agent.pyplugins/nemo-agents/src/nemo_agents_plugin/leaderboard/render.pyplugins/nemo-agents/src/nemo_agents_plugin/service.pyplugins/nemo-agents/src/nemo_agents_plugin/utils.pyplugins/nemo-agents/tests/unit/test_cli.pyplugins/nemo-agents/tests/unit/test_improvement_jobs.pyplugins/nemo-agents/tests/unit/test_optimize_agent_job.pyplugins/nemo-agents/tests/unit/test_optimize_skills_analyze_only.pyplugins/nemo-agents/tests/unit/test_service.pyplugins/nemo-agents/tests/unit/test_utils.pyplugins/nemo-agents/tests/unit/usage/test_usage_cli.pyplugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/backend.pyplugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/gpu.pyplugins/nemo-deployments/tests/unit/backends/docker/test_gpu.pyplugins/nemo-optimization/README.mdplugins/nemo-optimization/examples/hermes-optimize/.gitignoreplugins/nemo-optimization/examples/hermes-optimize/README.mdplugins/nemo-optimization/examples/hermes-optimize/agent.yamlplugins/nemo-optimization/examples/hermes-optimize/analyzer.inference-api.yamlplugins/nemo-optimization/examples/hermes-optimize/dataset-phishing.jsonplugins/nemo-optimization/examples/hermes-optimize/dataset.jsonplugins/nemo-optimization/examples/hermes-optimize/optimize.yamlplugins/nemo-optimization/examples/hermes-optimize/package.yamlplugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-chatonly.yamlplugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-mcp.e2e.yamlplugins/nemo-optimization/pyproject.tomlplugins/nemo-optimization/src/nemo_optimization/__init__.pyplugins/nemo-optimization/src/nemo_optimization/agents.pyplugins/nemo-optimization/src/nemo_optimization/backends/__init__.pyplugins/nemo-optimization/src/nemo_optimization/backends/ga/__init__.pyplugins/nemo-optimization/src/nemo_optimization/backends/ga/backend.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/__init__.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/artifacts.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/atif_metadata.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/backend.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/config_overlay.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/early_stop.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/fabric_trial.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/search_space.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/selection.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/study_driver.pyplugins/nemo-optimization/src/nemo_optimization/backends/protocol.pyplugins/nemo-optimization/src/nemo_optimization/config.pyplugins/nemo-optimization/src/nemo_optimization/fabric.pyplugins/nemo-optimization/src/nemo_optimization/jobs/__init__.pyplugins/nemo-optimization/src/nemo_optimization/jobs/optimize.pyplugins/nemo-optimization/src/nemo_optimization/preflight.pyplugins/nemo-optimization/src/nemo_optimization/registry.pyplugins/nemo-optimization/src/nemo_optimization/router.pyplugins/nemo-optimization/src/nemo_optimization/schemas/__init__.pyplugins/nemo-optimization/src/nemo_optimization/schemas/optimize.pyplugins/nemo-optimization/src/nemo_optimization/tasks/__init__.pyplugins/nemo-optimization/src/nemo_optimization/tasks/optimize.pyplugins/nemo-optimization/tests/conftest.pyplugins/nemo-optimization/tests/smoke_fabric_optimize_atif.pyplugins/nemo-optimization/tests/test_atif_metadata.pyplugins/nemo-optimization/tests/test_config_overlay.pyplugins/nemo-optimization/tests/test_fabric.pyplugins/nemo-optimization/tests/test_fabric_trial.pyplugins/nemo-optimization/tests/test_optimize_job.pyplugins/nemo-optimization/tests/test_router.pyplugins/nemo-optimization/tests/test_search_space.pyplugins/nemo-optimization/tests/test_selection.pyplugins/nemo-optimization/tests/test_study_driver.pypyproject.tomlservices/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/compiler.pyservices/core/models/tests/unit/controllers/backends/deployments_plugin/test_compiler.pythird_party/licenses.jsonlthird_party/osv-licenses.jsonthird_party/requirements-main.txt
💤 Files with no reviewable changes (3)
- plugins/nemo-agents/tests/unit/test_optimize_agent_job.py
- plugins/nemo-agents/src/nemo_agents_plugin/jobs/optimize_agent.py
- plugins/nemo-agents/tests/unit/test_utils.py
🚧 Files skipped from review as they are similar to previous changes (68)
- plugins/nemo-optimization/examples/hermes-optimize/dataset-phishing.json
- plugins/nemo-optimization/examples/hermes-optimize/dataset.json
- plugins/nemo-optimization/src/nemo_optimization/init.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/enums.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/init.py
- plugins/nemo-optimization/src/nemo_optimization/backends/init.py
- plugins/nemo-optimization/examples/hermes-optimize/package.yaml
- plugins/nemo-optimization/tests/test_atif_metadata.py
- plugins/nemo-optimization/examples/hermes-optimize/analyzer.inference-api.yaml
- plugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/backend.py
- packages/nemo_platform_plugin/tests/test_dispatcher.py
- plugins/nemo-optimization/src/nemo_optimization/jobs/init.py
- plugins/nemo-optimization/src/nemo_optimization/tasks/init.py
- plugins/nemo-optimization/tests/test_optimize_job.py
- plugins/nemo-optimization/examples/hermes-optimize/optimize.yaml
- packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_hook_loading.py
- plugins/nemo-optimization/src/nemo_optimization/schemas/init.py
- plugins/nemo-optimization/README.md
- plugins/nemo-optimization/src/nemo_optimization/backends/protocol.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/atif_metadata.py
- plugins/nemo-optimization/src/nemo_optimization/tasks/optimize.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hook_loading.py
- plugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-chatonly.yaml
- plugins/nemo-optimization/pyproject.toml
- plugins/nemo-optimization/tests/test_router.py
- plugins/nemo-agents/src/nemo_agents_plugin/utils.py
- plugins/nemo-optimization/src/nemo_optimization/schemas/optimize.py
- plugins/nemo-optimization/examples/hermes-optimize/agent.yaml
- plugins/nemo-optimization/src/nemo_optimization/registry.py
- plugins/nemo-optimization/tests/conftest.py
- plugins/nemo-optimization/src/nemo_optimization/fabric.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/early_stop.py
- plugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-mcp.e2e.yaml
- pyproject.toml
- plugins/nemo-optimization/src/nemo_optimization/agents.py
- plugins/nemo-optimization/src/nemo_optimization/preflight.py
- plugins/nemo-optimization/src/nemo_optimization/router.py
- plugins/nemo-optimization/examples/hermes-optimize/.gitignore
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/selection.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_runtime.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_evaluator.py
- plugins/nemo-optimization/tests/test_search_space.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/types.py
- plugins/nemo-optimization/tests/test_config_overlay.py
- plugins/nemo-optimization/src/nemo_optimization/backends/ga/init.py
- plugins/nemo-optimization/src/nemo_optimization/backends/ga/backend.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_defaults.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/study_driver.py
- plugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/gpu.py
- plugins/nemo-optimization/src/nemo_optimization/jobs/optimize.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/init.py
- plugins/nemo-agents/src/nemo_agents_plugin/cli.py
- plugins/nemo-agents/tests/unit/test_improvement_jobs.py
- packages/nemo_evaluator_sdk/pyproject.toml
- plugins/nemo-agents/pyproject.toml
- plugins/nemo-agents/src/nemo_agents_plugin/leaderboard/render.py
- services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/compiler.py
- plugins/nemo-agents/src/nemo_agents_plugin/service.py
- plugins/nemo-optimization/tests/test_fabric.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/init.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks.py
- plugins/nemo-optimization/tests/test_selection.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks_mcp_binding.py
- plugins/nemo-deployments/tests/unit/backends/docker/test_gpu.py
- plugins/nemo-agents/tests/unit/test_service.py
- docs/agents/optimization.mdx
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/metrics.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/search_space.py
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
♻️ Duplicate comments (1)
plugins/nemo-optimization/src/nemo_optimization/backends/optuna/artifacts.py (1)
62-64: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winBuild Pareto indexes in the filtered value index space.
_trial_valuesskips failed and pruned trials.pareto_indexesenumerates unfilteredstudy.trials. A skipped trial shifts later indexes and marks the wrong points as Pareto-optimal.Build values and Pareto indexes in one filtered pass. Add a regression test with a failed or pruned trial before a completed Pareto trial.
Proposed fix
- values = _trial_values(study.trials, len(metric_names)) pareto_numbers = _pareto_trial_numbers(study) - pareto_indexes = [index for index, trial in enumerate(study.trials) if trial.number in pareto_numbers] + values, pareto_indexes = _plot_points( + study.trials, len(metric_names), pareto_numbers + )🤖 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-optimization/src/nemo_optimization/backends/optuna/artifacts.py` around lines 62 - 64, Update the logic around _trial_values and pareto_indexes to filter failed and pruned trials once, then derive both metric values and Pareto indexes from that same filtered trial sequence. Preserve the existing Pareto membership check using trial.number, and add a regression test covering a failed or pruned trial before a completed Pareto trial.
🧹 Nitpick comments (1)
plugins/nemo-optimization/examples/hermes-optimize/README.md (1)
203-212: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a Next Steps section.
Add
## Next Stepswith links todocs/agents/optimization.mdxand the Fabric MCP workflow.🤖 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-optimization/examples/hermes-optimize/README.md` around lines 203 - 212, Add a “Next Steps” section to the README after the existing Notes, linking to docs/agents/optimization.mdx and the Fabric MCP workflow documentation.Source: Coding guidelines
🤖 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/index.mdx`:
- Around line 57-63: Remove the Optimize row from the “Legacy NAT-only commands”
table in the agents documentation, since `nemo agents optimize run` belongs to
the Fabric workflow. Add or update the surrounding documentation to direct users
to the Fabric optimization workflow, while keeping the NAT evaluation entry
unchanged.
In `@plugins/nemo-agents/tests/unit/usage/test_usage_cli.py`:
- Around line 210-216: Update the test around the usage CLI invocation to mock
the parser’s file-read operation so it raises PermissionError, rather than
relying on chmod(0o000) for bad. Remove the permission-bit manipulation and
retain the assertion coverage for handling the read failure through
runner.invoke.
---
Duplicate comments:
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/artifacts.py`:
- Around line 62-64: Update the logic around _trial_values and pareto_indexes to
filter failed and pruned trials once, then derive both metric values and Pareto
indexes from that same filtered trial sequence. Preserve the existing Pareto
membership check using trial.number, and add a regression test covering a failed
or pruned trial before a completed Pareto trial.
---
Nitpick comments:
In `@plugins/nemo-optimization/examples/hermes-optimize/README.md`:
- Around line 203-212: Add a “Next Steps” section to the README after the
existing Notes, linking to docs/agents/optimization.mdx and the Fabric MCP
workflow documentation.
🪄 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: d67004fd-d1d8-4754-ab90-7fd8a89cde5d
⛔ Files ignored due to path filters (12)
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/hook_loading.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/hooks.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/hooks_mcp_binding.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/runtime.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/enums.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/metrics/tunable_rag_defaults.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/metrics/tunable_rag_evaluator.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/metrics/types.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/values/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/values/metrics.pyis excluded by!sdk/**uv.lockis excluded by!**/*.lock
📒 Files selected for processing (93)
docs/agents/index.mdxdocs/agents/optimization.mdxpackages/nemo_evaluator_sdk/pyproject.tomlpackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/__init__.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hook_loading.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks_mcp_binding.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/enums.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_defaults.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_evaluator.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/types.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/__init__.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/metrics.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_hook_loading.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_runtime.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_mcp_run_binding_hook.pypackages/nemo_evaluator_sdk/tests/metrics/test_tunable_rag_evaluator.pypackages/nemo_platform/pyproject.tomlpackages/nemo_platform_plugin/tests/test_dispatcher.pyplugins/nemo-agents/openapi/openapi.yamlplugins/nemo-agents/pyproject.tomlplugins/nemo-agents/src/nemo_agents_plugin/cli.pyplugins/nemo-agents/src/nemo_agents_plugin/jobs/optimize_agent.pyplugins/nemo-agents/src/nemo_agents_plugin/leaderboard/render.pyplugins/nemo-agents/src/nemo_agents_plugin/service.pyplugins/nemo-agents/src/nemo_agents_plugin/utils.pyplugins/nemo-agents/tests/unit/test_cli.pyplugins/nemo-agents/tests/unit/test_improvement_jobs.pyplugins/nemo-agents/tests/unit/test_optimize_agent_job.pyplugins/nemo-agents/tests/unit/test_optimize_skills_analyze_only.pyplugins/nemo-agents/tests/unit/test_service.pyplugins/nemo-agents/tests/unit/test_utils.pyplugins/nemo-agents/tests/unit/usage/test_usage_cli.pyplugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/backend.pyplugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/gpu.pyplugins/nemo-deployments/tests/unit/backends/docker/test_gpu.pyplugins/nemo-optimization/README.mdplugins/nemo-optimization/examples/hermes-optimize/.gitignoreplugins/nemo-optimization/examples/hermes-optimize/README.mdplugins/nemo-optimization/examples/hermes-optimize/agent.yamlplugins/nemo-optimization/examples/hermes-optimize/analyzer.inference-api.yamlplugins/nemo-optimization/examples/hermes-optimize/dataset-phishing.jsonplugins/nemo-optimization/examples/hermes-optimize/dataset.jsonplugins/nemo-optimization/examples/hermes-optimize/optimize.yamlplugins/nemo-optimization/examples/hermes-optimize/package.yamlplugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-chatonly.yamlplugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-mcp.e2e.yamlplugins/nemo-optimization/pyproject.tomlplugins/nemo-optimization/src/nemo_optimization/__init__.pyplugins/nemo-optimization/src/nemo_optimization/agents.pyplugins/nemo-optimization/src/nemo_optimization/backends/__init__.pyplugins/nemo-optimization/src/nemo_optimization/backends/ga/__init__.pyplugins/nemo-optimization/src/nemo_optimization/backends/ga/backend.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/__init__.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/artifacts.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/atif_metadata.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/backend.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/config_overlay.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/early_stop.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/fabric_trial.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/search_space.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/selection.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/study_driver.pyplugins/nemo-optimization/src/nemo_optimization/backends/protocol.pyplugins/nemo-optimization/src/nemo_optimization/config.pyplugins/nemo-optimization/src/nemo_optimization/fabric.pyplugins/nemo-optimization/src/nemo_optimization/jobs/__init__.pyplugins/nemo-optimization/src/nemo_optimization/jobs/optimize.pyplugins/nemo-optimization/src/nemo_optimization/preflight.pyplugins/nemo-optimization/src/nemo_optimization/registry.pyplugins/nemo-optimization/src/nemo_optimization/router.pyplugins/nemo-optimization/src/nemo_optimization/schemas/__init__.pyplugins/nemo-optimization/src/nemo_optimization/schemas/optimize.pyplugins/nemo-optimization/src/nemo_optimization/tasks/__init__.pyplugins/nemo-optimization/src/nemo_optimization/tasks/optimize.pyplugins/nemo-optimization/tests/conftest.pyplugins/nemo-optimization/tests/smoke_fabric_optimize_atif.pyplugins/nemo-optimization/tests/test_atif_metadata.pyplugins/nemo-optimization/tests/test_config_overlay.pyplugins/nemo-optimization/tests/test_fabric.pyplugins/nemo-optimization/tests/test_fabric_trial.pyplugins/nemo-optimization/tests/test_optimize_job.pyplugins/nemo-optimization/tests/test_router.pyplugins/nemo-optimization/tests/test_search_space.pyplugins/nemo-optimization/tests/test_selection.pyplugins/nemo-optimization/tests/test_study_driver.pypyproject.tomlservices/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/compiler.pyservices/core/models/tests/unit/controllers/backends/deployments_plugin/test_compiler.pythird_party/licenses.jsonlthird_party/osv-licenses.jsonthird_party/requirements-main.txt
💤 Files with no reviewable changes (3)
- plugins/nemo-agents/tests/unit/test_optimize_agent_job.py
- plugins/nemo-agents/src/nemo_agents_plugin/jobs/optimize_agent.py
- plugins/nemo-agents/tests/unit/test_utils.py
🚧 Files skipped from review as they are similar to previous changes (68)
- plugins/nemo-optimization/examples/hermes-optimize/dataset-phishing.json
- plugins/nemo-optimization/examples/hermes-optimize/dataset.json
- plugins/nemo-optimization/src/nemo_optimization/init.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/enums.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/init.py
- plugins/nemo-optimization/src/nemo_optimization/backends/init.py
- plugins/nemo-optimization/examples/hermes-optimize/package.yaml
- plugins/nemo-optimization/tests/test_atif_metadata.py
- plugins/nemo-optimization/examples/hermes-optimize/analyzer.inference-api.yaml
- plugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/backend.py
- packages/nemo_platform_plugin/tests/test_dispatcher.py
- plugins/nemo-optimization/src/nemo_optimization/jobs/init.py
- plugins/nemo-optimization/src/nemo_optimization/tasks/init.py
- plugins/nemo-optimization/tests/test_optimize_job.py
- plugins/nemo-optimization/examples/hermes-optimize/optimize.yaml
- packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_hook_loading.py
- plugins/nemo-optimization/src/nemo_optimization/schemas/init.py
- plugins/nemo-optimization/README.md
- plugins/nemo-optimization/src/nemo_optimization/backends/protocol.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/atif_metadata.py
- plugins/nemo-optimization/src/nemo_optimization/tasks/optimize.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hook_loading.py
- plugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-chatonly.yaml
- plugins/nemo-optimization/pyproject.toml
- plugins/nemo-optimization/tests/test_router.py
- plugins/nemo-agents/src/nemo_agents_plugin/utils.py
- plugins/nemo-optimization/src/nemo_optimization/schemas/optimize.py
- plugins/nemo-optimization/examples/hermes-optimize/agent.yaml
- plugins/nemo-optimization/src/nemo_optimization/registry.py
- plugins/nemo-optimization/tests/conftest.py
- plugins/nemo-optimization/src/nemo_optimization/fabric.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/early_stop.py
- plugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-mcp.e2e.yaml
- pyproject.toml
- plugins/nemo-optimization/src/nemo_optimization/agents.py
- plugins/nemo-optimization/src/nemo_optimization/preflight.py
- plugins/nemo-optimization/src/nemo_optimization/router.py
- plugins/nemo-optimization/examples/hermes-optimize/.gitignore
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/selection.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_runtime.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_evaluator.py
- plugins/nemo-optimization/tests/test_search_space.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/types.py
- plugins/nemo-optimization/tests/test_config_overlay.py
- plugins/nemo-optimization/src/nemo_optimization/backends/ga/init.py
- plugins/nemo-optimization/src/nemo_optimization/backends/ga/backend.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_defaults.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/study_driver.py
- plugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/gpu.py
- plugins/nemo-optimization/src/nemo_optimization/jobs/optimize.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/init.py
- plugins/nemo-agents/src/nemo_agents_plugin/cli.py
- plugins/nemo-agents/tests/unit/test_improvement_jobs.py
- packages/nemo_evaluator_sdk/pyproject.toml
- plugins/nemo-agents/pyproject.toml
- plugins/nemo-agents/src/nemo_agents_plugin/leaderboard/render.py
- services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/compiler.py
- plugins/nemo-agents/src/nemo_agents_plugin/service.py
- plugins/nemo-optimization/tests/test_fabric.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/init.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks.py
- plugins/nemo-optimization/tests/test_selection.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks_mcp_binding.py
- plugins/nemo-deployments/tests/unit/backends/docker/test_gpu.py
- plugins/nemo-agents/tests/unit/test_service.py
- docs/agents/optimization.mdx
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/metrics.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/search_space.py
🛑 Comments failed to post (1)
plugins/nemo-agents/tests/unit/usage/test_usage_cli.py (1)
210-216: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Mock the read failure.
Line 212 does not block reads for privileged runners. Windows also does not enforce these mode bits consistently. Mock the parser file-read operation to raise
PermissionErrorinstead.🤖 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-agents/tests/unit/usage/test_usage_cli.py` around lines 210 - 216, Update the test around the usage CLI invocation to mock the parser’s file-read operation so it raises PermissionError, rather than relying on chmod(0o000) for bad. Remove the permission-bit manipulation and retain the assertion coverage for handling the read failure through runner.invoke.
soluwalana
force-pushed
the
aalgo-277/solu
branch
from
August 5, 2026 18:00
19fa353 to
d7d4657
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/nemo-optimization/examples/hermes-optimize/README.md (1)
203-217: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a final Next Steps section.
The README ends with
Notes. AddNext Stepswith cross-links to the related Optimize Agents documentation.🤖 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-optimization/examples/hermes-optimize/README.md` around lines 203 - 217, Add a final “Next Steps” section after “Notes” in the README, including cross-links to the related Optimize Agents documentation. Keep the existing Notes content unchanged and use the repository’s established documentation link targets where available.Source: Coding guidelines
♻️ Duplicate comments (2)
plugins/nemo-optimization/examples/hermes-optimize/README.md (1)
71-114: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse tab sets for the CLI and SDK variants.
These parallel recipes use repeated
CLIandPython SDKheadings. They violate the tab-set requirement and trigger MD024. Put each recipe pair in a supported tab set, or link to the tabbed documentation page instead.Also applies to: 144-196
🤖 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-optimization/examples/hermes-optimize/README.md` around lines 71 - 114, Update the “Clean chat-only run” documentation and the corresponding recipe section around the referenced later block to wrap each CLI/Python SDK pair in the repository’s supported tab-set format. Replace the repeated standalone CLI and Python SDK headings with tab labels consistent with existing tabbed documentation, while preserving both command and SDK examples unchanged.Sources: Coding guidelines, Linters/SAST tools
plugins/nemo-optimization/src/nemo_optimization/backends/optuna/artifacts.py (1)
62-64: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep Pareto indexes in the filtered plot index space.
_trial_valuesskips failed, pruned, and incomplete trials. Line 64 indexes unfilteredstudy.trials. A skipped trial before a Pareto trial marks the wrong plot point. Build values and Pareto indexes in one filtered pass.Proposed fix
- values = _trial_values(study.trials, len(metric_names)) pareto_numbers = _pareto_trial_numbers(study) - pareto_indexes = [index for index, trial in enumerate(study.trials) if trial.number in pareto_numbers] + values, pareto_indexes = _plot_points(study.trials, len(metric_names), pareto_numbers)def _plot_points( trials: Sequence[optuna.trial.FrozenTrial], n_metrics: int, pareto_numbers: set[int], ) -> tuple[list[list[float]], list[int]]: values: list[list[float]] = [] pareto_indexes: list[int] = [] for trial in trials: trial_values = list(trial.values or ([] if trial.value is None else [trial.value])) if len(trial_values) != n_metrics: continue if trial.number in pareto_numbers: pareto_indexes.append(len(values)) values.append([float(value) for value in trial_values]) return values, pareto_indexes🤖 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-optimization/src/nemo_optimization/backends/optuna/artifacts.py` around lines 62 - 64, Update the plotting preparation around _trial_values and pareto_indexes to use one filtered pass over study.trials, retaining only complete trials with exactly len(metric_names) values. Build values and append each Pareto trial’s index based on the filtered values list position, then use both results for plotting so skipped trials cannot shift Pareto indexes.
🧹 Nitpick comments (3)
plugins/nemo-optimization/src/nemo_optimization/backends/optuna/fabric_trial.py (1)
47-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the redundant
isinstancere-checks onfabric_eval.Line 47 already guarantees
fabric_evalis aMapping(or{}). Theisinstance(fabric_eval, Mapping)guards on Lines 54, 56, and 58 can never be false.♻️ Proposed change
- fabric_eval = self._eval_config.get("fabric") if isinstance(self._eval_config.get("fabric"), Mapping) else {} + raw_fabric = self._eval_config.get("fabric") + fabric_eval: Mapping[str, Any] = raw_fabric if isinstance(raw_fabric, Mapping) else {} run_hook_spec = self._eval_config.get("run_hook") try: self._task_hook = load_fabric_task_hook(run_hook_spec if isinstance(run_hook_spec, Mapping) else None) except FabricTaskHookLoadError as exc: raise StudyDriverError(str(exc)) from exc - self._fabric_base_dir = _optional_path( - fabric_eval.get("base_dir") if isinstance(fabric_eval, Mapping) else None - ) - self._timeout_s = int(fabric_eval.get("timeout_s", 600) if isinstance(fabric_eval, Mapping) else 600) - self._capture_trajectory = bool( - fabric_eval.get("capture_trajectory", True) if isinstance(fabric_eval, Mapping) else True - ) + self._fabric_base_dir = _optional_path(fabric_eval.get("base_dir")) + self._timeout_s = int(fabric_eval.get("timeout_s", 600)) + self._capture_trajectory = bool(fabric_eval.get("capture_trajectory", True))🤖 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-optimization/src/nemo_optimization/backends/optuna/fabric_trial.py` around lines 47 - 59, Remove the redundant isinstance(fabric_eval, Mapping) checks in the _fabric_base_dir, _timeout_s, and _capture_trajectory initialization within the constructor. Use fabric_eval directly for these .get calls, preserving the existing defaults and behavior.plugins/nemo-agents/tests/unit/test_optimize_skills_analyze_only.py (1)
231-251: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd return and parameter type hints.
The repository guideline requires concrete type hints.
_agents_cli_with_jobshas no return type and_guard_messagehas an untypedresultparameter.♻️ Proposed change
-def _agents_cli_with_jobs(): +def _agents_cli_with_jobs() -> typer.Typer:-def _guard_message(result) -> str: +def _guard_message(result: Result) -> str:Import
typerandfrom click.testing import Resultat module scope.As per coding guidelines: "Prefer concrete type hints over string-based type hints".
🤖 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-agents/tests/unit/test_optimize_skills_analyze_only.py` around lines 231 - 251, Update _agents_cli_with_jobs with a concrete return type and annotate _guard_message’s result parameter with click.testing.Result; add the required module-level imports for these types, including typer if needed for the CLI return annotation, while preserving both helpers’ behavior.Source: Coding guidelines
plugins/nemo-optimization/src/nemo_optimization/backends/optuna/backend.py (1)
89-89: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAnnotate
resultandoutput_dir.
_study_debug_payload(result)and_build_trial_evaluator(..., output_dir)have untyped parameters and_build_trial_evaluatorhas no return type. Use the concrete study-result type andPath.As per coding guidelines: "Prefer concrete type hints over string-based type hints".
Also applies to: 119-125
🤖 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-optimization/src/nemo_optimization/backends/optuna/backend.py` at line 89, Update _study_debug_payload to annotate result with the concrete study-result type, and update _build_trial_evaluator to annotate output_dir as Path and add its concrete return type. Use direct imported type hints rather than string-based annotations, preserving the existing behavior.Source: Coding guidelines
🤖 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-agents/tests/unit/usage/test_usage_cli.py`:
- Around line 204-216: Update
test_usage_show_unreadable_result_json_exits_cleanly to mock the parser’s
file-read operation so it raises PermissionError instead of relying on
chmod(0o000); preserve the existing runner invocation and CLI assertions
unchanged.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/fabric_trial.py`:
- Line 62: Update the parallelism initialization in the trial setup to validate
that the eval_config “general” value is a mapping before reading
max_concurrency. Reuse the existing validation pattern from the guard around
line 240, and raise StudyDriverError for null or non-mapping values instead of
calling .get on them.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/selection.py`:
- Around line 42-44: Update the harmonic branch in the selection logic around
normalized_mode to compute the harmonic mean over utilities (1 - norm), then
select the trial with the maximum utility harmonic mean instead of minimizing
hmean(norm). Add a regression test covering [0.0, 1.0] versus [0.4, 0.4],
asserting the balanced compromise is selected.
In `@plugins/nemo-optimization/tests/smoke_fabric_optimize_atif.py`:
- Around line 130-131: Update the assertion message immediately after
constructing atif_path to reference the same trace_ref used to create and
validate that path, rather than the stale entry loop variable. Keep the is_file
assertion behavior unchanged.
---
Outside diff comments:
In `@plugins/nemo-optimization/examples/hermes-optimize/README.md`:
- Around line 203-217: Add a final “Next Steps” section after “Notes” in the
README, including cross-links to the related Optimize Agents documentation. Keep
the existing Notes content unchanged and use the repository’s established
documentation link targets where available.
---
Duplicate comments:
In `@plugins/nemo-optimization/examples/hermes-optimize/README.md`:
- Around line 71-114: Update the “Clean chat-only run” documentation and the
corresponding recipe section around the referenced later block to wrap each
CLI/Python SDK pair in the repository’s supported tab-set format. Replace the
repeated standalone CLI and Python SDK headings with tab labels consistent with
existing tabbed documentation, while preserving both command and SDK examples
unchanged.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/artifacts.py`:
- Around line 62-64: Update the plotting preparation around _trial_values and
pareto_indexes to use one filtered pass over study.trials, retaining only
complete trials with exactly len(metric_names) values. Build values and append
each Pareto trial’s index based on the filtered values list position, then use
both results for plotting so skipped trials cannot shift Pareto indexes.
---
Nitpick comments:
In `@plugins/nemo-agents/tests/unit/test_optimize_skills_analyze_only.py`:
- Around line 231-251: Update _agents_cli_with_jobs with a concrete return type
and annotate _guard_message’s result parameter with click.testing.Result; add
the required module-level imports for these types, including typer if needed for
the CLI return annotation, while preserving both helpers’ behavior.
In `@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/backend.py`:
- Line 89: Update _study_debug_payload to annotate result with the concrete
study-result type, and update _build_trial_evaluator to annotate output_dir as
Path and add its concrete return type. Use direct imported type hints rather
than string-based annotations, preserving the existing behavior.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/fabric_trial.py`:
- Around line 47-59: Remove the redundant isinstance(fabric_eval, Mapping)
checks in the _fabric_base_dir, _timeout_s, and _capture_trajectory
initialization within the constructor. Use fabric_eval directly for these .get
calls, preserving the existing defaults and behavior.
🪄 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: 3851d6b3-65bb-459f-b436-6506f341d4ec
⛔ Files ignored due to path filters (12)
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/hook_loading.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/hooks.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/hooks_mcp_binding.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/runtime.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/enums.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/metrics/tunable_rag_defaults.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/metrics/tunable_rag_evaluator.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/metrics/types.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/values/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/values/metrics.pyis excluded by!sdk/**uv.lockis excluded by!**/*.lock
📒 Files selected for processing (93)
docs/agents/index.mdxdocs/agents/optimization.mdxpackages/nemo_evaluator_sdk/pyproject.tomlpackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/__init__.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hook_loading.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks_mcp_binding.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/enums.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_defaults.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_evaluator.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/types.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/__init__.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/metrics.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_hook_loading.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_runtime.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_mcp_run_binding_hook.pypackages/nemo_evaluator_sdk/tests/metrics/test_tunable_rag_evaluator.pypackages/nemo_platform/pyproject.tomlpackages/nemo_platform_plugin/tests/test_dispatcher.pyplugins/nemo-agents/openapi/openapi.yamlplugins/nemo-agents/pyproject.tomlplugins/nemo-agents/src/nemo_agents_plugin/cli.pyplugins/nemo-agents/src/nemo_agents_plugin/jobs/optimize_agent.pyplugins/nemo-agents/src/nemo_agents_plugin/leaderboard/render.pyplugins/nemo-agents/src/nemo_agents_plugin/service.pyplugins/nemo-agents/src/nemo_agents_plugin/utils.pyplugins/nemo-agents/tests/unit/test_cli.pyplugins/nemo-agents/tests/unit/test_improvement_jobs.pyplugins/nemo-agents/tests/unit/test_optimize_agent_job.pyplugins/nemo-agents/tests/unit/test_optimize_skills_analyze_only.pyplugins/nemo-agents/tests/unit/test_service.pyplugins/nemo-agents/tests/unit/test_utils.pyplugins/nemo-agents/tests/unit/usage/test_usage_cli.pyplugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/backend.pyplugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/gpu.pyplugins/nemo-deployments/tests/unit/backends/docker/test_gpu.pyplugins/nemo-optimization/README.mdplugins/nemo-optimization/examples/hermes-optimize/.gitignoreplugins/nemo-optimization/examples/hermes-optimize/README.mdplugins/nemo-optimization/examples/hermes-optimize/agent.yamlplugins/nemo-optimization/examples/hermes-optimize/analyzer.inference-api.yamlplugins/nemo-optimization/examples/hermes-optimize/dataset-phishing.jsonplugins/nemo-optimization/examples/hermes-optimize/dataset.jsonplugins/nemo-optimization/examples/hermes-optimize/optimize.yamlplugins/nemo-optimization/examples/hermes-optimize/package.yamlplugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-chatonly.yamlplugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-mcp.e2e.yamlplugins/nemo-optimization/pyproject.tomlplugins/nemo-optimization/src/nemo_optimization/__init__.pyplugins/nemo-optimization/src/nemo_optimization/agents.pyplugins/nemo-optimization/src/nemo_optimization/backends/__init__.pyplugins/nemo-optimization/src/nemo_optimization/backends/ga/__init__.pyplugins/nemo-optimization/src/nemo_optimization/backends/ga/backend.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/__init__.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/artifacts.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/atif_metadata.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/backend.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/config_overlay.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/early_stop.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/fabric_trial.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/search_space.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/selection.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/study_driver.pyplugins/nemo-optimization/src/nemo_optimization/backends/protocol.pyplugins/nemo-optimization/src/nemo_optimization/config.pyplugins/nemo-optimization/src/nemo_optimization/fabric.pyplugins/nemo-optimization/src/nemo_optimization/jobs/__init__.pyplugins/nemo-optimization/src/nemo_optimization/jobs/optimize.pyplugins/nemo-optimization/src/nemo_optimization/preflight.pyplugins/nemo-optimization/src/nemo_optimization/registry.pyplugins/nemo-optimization/src/nemo_optimization/router.pyplugins/nemo-optimization/src/nemo_optimization/schemas/__init__.pyplugins/nemo-optimization/src/nemo_optimization/schemas/optimize.pyplugins/nemo-optimization/src/nemo_optimization/tasks/__init__.pyplugins/nemo-optimization/src/nemo_optimization/tasks/optimize.pyplugins/nemo-optimization/tests/conftest.pyplugins/nemo-optimization/tests/smoke_fabric_optimize_atif.pyplugins/nemo-optimization/tests/test_atif_metadata.pyplugins/nemo-optimization/tests/test_config_overlay.pyplugins/nemo-optimization/tests/test_fabric.pyplugins/nemo-optimization/tests/test_fabric_trial.pyplugins/nemo-optimization/tests/test_optimize_job.pyplugins/nemo-optimization/tests/test_router.pyplugins/nemo-optimization/tests/test_search_space.pyplugins/nemo-optimization/tests/test_selection.pyplugins/nemo-optimization/tests/test_study_driver.pypyproject.tomlservices/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/compiler.pyservices/core/models/tests/unit/controllers/backends/deployments_plugin/test_compiler.pythird_party/licenses.jsonlthird_party/osv-licenses.jsonthird_party/requirements-main.txt
💤 Files with no reviewable changes (3)
- plugins/nemo-agents/src/nemo_agents_plugin/jobs/optimize_agent.py
- plugins/nemo-agents/tests/unit/test_optimize_agent_job.py
- plugins/nemo-agents/tests/unit/test_utils.py
🚧 Files skipped from review as they are similar to previous changes (71)
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/init.py
- packages/nemo_evaluator_sdk/pyproject.toml
- plugins/nemo-agents/tests/unit/test_service.py
- pyproject.toml
- packages/nemo_platform_plugin/tests/test_dispatcher.py
- plugins/nemo-optimization/examples/hermes-optimize/dataset.json
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/init.py
- services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/compiler.py
- plugins/nemo-optimization/src/nemo_optimization/init.py
- plugins/nemo-agents/tests/unit/test_improvement_jobs.py
- plugins/nemo-agents/src/nemo_agents_plugin/leaderboard/render.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_hook_loading.py
- plugins/nemo-optimization/tests/test_search_space.py
- plugins/nemo-optimization/examples/hermes-optimize/package.yaml
- plugins/nemo-optimization/examples/hermes-optimize/analyzer.inference-api.yaml
- plugins/nemo-optimization/examples/hermes-optimize/dataset-phishing.json
- plugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-mcp.e2e.yaml
- plugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/backend.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/atif_metadata.py
- plugins/nemo-agents/src/nemo_agents_plugin/utils.py
- plugins/nemo-optimization/README.md
- plugins/nemo-optimization/examples/hermes-optimize/optimize.yaml
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/metrics.py
- third_party/licenses.jsonl
- packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_runtime.py
- plugins/nemo-optimization/examples/hermes-optimize/.gitignore
- plugins/nemo-optimization/tests/test_selection.py
- plugins/nemo-agents/src/nemo_agents_plugin/cli.py
- plugins/nemo-optimization/src/nemo_optimization/backends/init.py
- plugins/nemo-optimization/pyproject.toml
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/enums.py
- plugins/nemo-agents/src/nemo_agents_plugin/service.py
- plugins/nemo-optimization/examples/hermes-optimize/agent.yaml
- plugins/nemo-optimization/src/nemo_optimization/backends/ga/backend.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/init.py
- plugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/gpu.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks.py
- plugins/nemo-optimization/src/nemo_optimization/agents.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hook_loading.py
- plugins/nemo-optimization/src/nemo_optimization/config.py
- plugins/nemo-optimization/src/nemo_optimization/backends/ga/init.py
- plugins/nemo-optimization/src/nemo_optimization/jobs/init.py
- packages/nemo_platform/pyproject.toml
- plugins/nemo-optimization/src/nemo_optimization/tasks/init.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_defaults.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_evaluator.py
- plugins/nemo-optimization/tests/test_config_overlay.py
- plugins/nemo-optimization/tests/test_atif_metadata.py
- plugins/nemo-optimization/src/nemo_optimization/schemas/init.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/types.py
- plugins/nemo-optimization/src/nemo_optimization/schemas/optimize.py
- docs/agents/optimization.mdx
- plugins/nemo-optimization/src/nemo_optimization/tasks/optimize.py
- plugins/nemo-optimization/src/nemo_optimization/backends/protocol.py
- plugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-chatonly.yaml
- plugins/nemo-optimization/tests/test_fabric.py
- plugins/nemo-agents/pyproject.toml
- plugins/nemo-optimization/tests/test_optimize_job.py
- plugins/nemo-optimization/src/nemo_optimization/registry.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/early_stop.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/config_overlay.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks_mcp_binding.py
- plugins/nemo-optimization/tests/test_router.py
- plugins/nemo-optimization/src/nemo_optimization/fabric.py
- plugins/nemo-optimization/src/nemo_optimization/preflight.py
- plugins/nemo-deployments/tests/unit/backends/docker/test_gpu.py
- third_party/requirements-main.txt
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/search_space.py
- plugins/nemo-optimization/src/nemo_optimization/router.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/study_driver.py
- plugins/nemo-optimization/src/nemo_optimization/jobs/optimize.py
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/nemo-optimization/examples/hermes-optimize/README.md (1)
203-217: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a final Next Steps section.
The README ends with
Notes. AddNext Stepswith cross-links to the related Optimize Agents documentation.🤖 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-optimization/examples/hermes-optimize/README.md` around lines 203 - 217, Add a final “Next Steps” section after “Notes” in the README, including cross-links to the related Optimize Agents documentation. Keep the existing Notes content unchanged and use the repository’s established documentation link targets where available.Source: Coding guidelines
♻️ Duplicate comments (2)
plugins/nemo-optimization/examples/hermes-optimize/README.md (1)
71-114: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse tab sets for the CLI and SDK variants.
These parallel recipes use repeated
CLIandPython SDKheadings. They violate the tab-set requirement and trigger MD024. Put each recipe pair in a supported tab set, or link to the tabbed documentation page instead.Also applies to: 144-196
🤖 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-optimization/examples/hermes-optimize/README.md` around lines 71 - 114, Update the “Clean chat-only run” documentation and the corresponding recipe section around the referenced later block to wrap each CLI/Python SDK pair in the repository’s supported tab-set format. Replace the repeated standalone CLI and Python SDK headings with tab labels consistent with existing tabbed documentation, while preserving both command and SDK examples unchanged.Sources: Coding guidelines, Linters/SAST tools
plugins/nemo-optimization/src/nemo_optimization/backends/optuna/artifacts.py (1)
62-64: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep Pareto indexes in the filtered plot index space.
_trial_valuesskips failed, pruned, and incomplete trials. Line 64 indexes unfilteredstudy.trials. A skipped trial before a Pareto trial marks the wrong plot point. Build values and Pareto indexes in one filtered pass.Proposed fix
- values = _trial_values(study.trials, len(metric_names)) pareto_numbers = _pareto_trial_numbers(study) - pareto_indexes = [index for index, trial in enumerate(study.trials) if trial.number in pareto_numbers] + values, pareto_indexes = _plot_points(study.trials, len(metric_names), pareto_numbers)def _plot_points( trials: Sequence[optuna.trial.FrozenTrial], n_metrics: int, pareto_numbers: set[int], ) -> tuple[list[list[float]], list[int]]: values: list[list[float]] = [] pareto_indexes: list[int] = [] for trial in trials: trial_values = list(trial.values or ([] if trial.value is None else [trial.value])) if len(trial_values) != n_metrics: continue if trial.number in pareto_numbers: pareto_indexes.append(len(values)) values.append([float(value) for value in trial_values]) return values, pareto_indexes🤖 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-optimization/src/nemo_optimization/backends/optuna/artifacts.py` around lines 62 - 64, Update the plotting preparation around _trial_values and pareto_indexes to use one filtered pass over study.trials, retaining only complete trials with exactly len(metric_names) values. Build values and append each Pareto trial’s index based on the filtered values list position, then use both results for plotting so skipped trials cannot shift Pareto indexes.
🧹 Nitpick comments (3)
plugins/nemo-optimization/src/nemo_optimization/backends/optuna/fabric_trial.py (1)
47-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the redundant
isinstancere-checks onfabric_eval.Line 47 already guarantees
fabric_evalis aMapping(or{}). Theisinstance(fabric_eval, Mapping)guards on Lines 54, 56, and 58 can never be false.♻️ Proposed change
- fabric_eval = self._eval_config.get("fabric") if isinstance(self._eval_config.get("fabric"), Mapping) else {} + raw_fabric = self._eval_config.get("fabric") + fabric_eval: Mapping[str, Any] = raw_fabric if isinstance(raw_fabric, Mapping) else {} run_hook_spec = self._eval_config.get("run_hook") try: self._task_hook = load_fabric_task_hook(run_hook_spec if isinstance(run_hook_spec, Mapping) else None) except FabricTaskHookLoadError as exc: raise StudyDriverError(str(exc)) from exc - self._fabric_base_dir = _optional_path( - fabric_eval.get("base_dir") if isinstance(fabric_eval, Mapping) else None - ) - self._timeout_s = int(fabric_eval.get("timeout_s", 600) if isinstance(fabric_eval, Mapping) else 600) - self._capture_trajectory = bool( - fabric_eval.get("capture_trajectory", True) if isinstance(fabric_eval, Mapping) else True - ) + self._fabric_base_dir = _optional_path(fabric_eval.get("base_dir")) + self._timeout_s = int(fabric_eval.get("timeout_s", 600)) + self._capture_trajectory = bool(fabric_eval.get("capture_trajectory", True))🤖 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-optimization/src/nemo_optimization/backends/optuna/fabric_trial.py` around lines 47 - 59, Remove the redundant isinstance(fabric_eval, Mapping) checks in the _fabric_base_dir, _timeout_s, and _capture_trajectory initialization within the constructor. Use fabric_eval directly for these .get calls, preserving the existing defaults and behavior.plugins/nemo-agents/tests/unit/test_optimize_skills_analyze_only.py (1)
231-251: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd return and parameter type hints.
The repository guideline requires concrete type hints.
_agents_cli_with_jobshas no return type and_guard_messagehas an untypedresultparameter.♻️ Proposed change
-def _agents_cli_with_jobs(): +def _agents_cli_with_jobs() -> typer.Typer:-def _guard_message(result) -> str: +def _guard_message(result: Result) -> str:Import
typerandfrom click.testing import Resultat module scope.As per coding guidelines: "Prefer concrete type hints over string-based type hints".
🤖 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-agents/tests/unit/test_optimize_skills_analyze_only.py` around lines 231 - 251, Update _agents_cli_with_jobs with a concrete return type and annotate _guard_message’s result parameter with click.testing.Result; add the required module-level imports for these types, including typer if needed for the CLI return annotation, while preserving both helpers’ behavior.Source: Coding guidelines
plugins/nemo-optimization/src/nemo_optimization/backends/optuna/backend.py (1)
89-89: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAnnotate
resultandoutput_dir.
_study_debug_payload(result)and_build_trial_evaluator(..., output_dir)have untyped parameters and_build_trial_evaluatorhas no return type. Use the concrete study-result type andPath.As per coding guidelines: "Prefer concrete type hints over string-based type hints".
Also applies to: 119-125
🤖 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-optimization/src/nemo_optimization/backends/optuna/backend.py` at line 89, Update _study_debug_payload to annotate result with the concrete study-result type, and update _build_trial_evaluator to annotate output_dir as Path and add its concrete return type. Use direct imported type hints rather than string-based annotations, preserving the existing behavior.Source: Coding guidelines
🤖 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-agents/tests/unit/usage/test_usage_cli.py`:
- Around line 204-216: Update
test_usage_show_unreadable_result_json_exits_cleanly to mock the parser’s
file-read operation so it raises PermissionError instead of relying on
chmod(0o000); preserve the existing runner invocation and CLI assertions
unchanged.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/fabric_trial.py`:
- Line 62: Update the parallelism initialization in the trial setup to validate
that the eval_config “general” value is a mapping before reading
max_concurrency. Reuse the existing validation pattern from the guard around
line 240, and raise StudyDriverError for null or non-mapping values instead of
calling .get on them.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/selection.py`:
- Around line 42-44: Update the harmonic branch in the selection logic around
normalized_mode to compute the harmonic mean over utilities (1 - norm), then
select the trial with the maximum utility harmonic mean instead of minimizing
hmean(norm). Add a regression test covering [0.0, 1.0] versus [0.4, 0.4],
asserting the balanced compromise is selected.
In `@plugins/nemo-optimization/tests/smoke_fabric_optimize_atif.py`:
- Around line 130-131: Update the assertion message immediately after
constructing atif_path to reference the same trace_ref used to create and
validate that path, rather than the stale entry loop variable. Keep the is_file
assertion behavior unchanged.
---
Outside diff comments:
In `@plugins/nemo-optimization/examples/hermes-optimize/README.md`:
- Around line 203-217: Add a final “Next Steps” section after “Notes” in the
README, including cross-links to the related Optimize Agents documentation. Keep
the existing Notes content unchanged and use the repository’s established
documentation link targets where available.
---
Duplicate comments:
In `@plugins/nemo-optimization/examples/hermes-optimize/README.md`:
- Around line 71-114: Update the “Clean chat-only run” documentation and the
corresponding recipe section around the referenced later block to wrap each
CLI/Python SDK pair in the repository’s supported tab-set format. Replace the
repeated standalone CLI and Python SDK headings with tab labels consistent with
existing tabbed documentation, while preserving both command and SDK examples
unchanged.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/artifacts.py`:
- Around line 62-64: Update the plotting preparation around _trial_values and
pareto_indexes to use one filtered pass over study.trials, retaining only
complete trials with exactly len(metric_names) values. Build values and append
each Pareto trial’s index based on the filtered values list position, then use
both results for plotting so skipped trials cannot shift Pareto indexes.
---
Nitpick comments:
In `@plugins/nemo-agents/tests/unit/test_optimize_skills_analyze_only.py`:
- Around line 231-251: Update _agents_cli_with_jobs with a concrete return type
and annotate _guard_message’s result parameter with click.testing.Result; add
the required module-level imports for these types, including typer if needed for
the CLI return annotation, while preserving both helpers’ behavior.
In `@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/backend.py`:
- Line 89: Update _study_debug_payload to annotate result with the concrete
study-result type, and update _build_trial_evaluator to annotate output_dir as
Path and add its concrete return type. Use direct imported type hints rather
than string-based annotations, preserving the existing behavior.
In
`@plugins/nemo-optimization/src/nemo_optimization/backends/optuna/fabric_trial.py`:
- Around line 47-59: Remove the redundant isinstance(fabric_eval, Mapping)
checks in the _fabric_base_dir, _timeout_s, and _capture_trajectory
initialization within the constructor. Use fabric_eval directly for these .get
calls, preserving the existing defaults and behavior.
🪄 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: 3851d6b3-65bb-459f-b436-6506f341d4ec
⛔ Files ignored due to path filters (12)
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/hook_loading.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/hooks.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/hooks_mcp_binding.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/runtime.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/enums.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/metrics/tunable_rag_defaults.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/metrics/tunable_rag_evaluator.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/metrics/types.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/values/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/values/metrics.pyis excluded by!sdk/**uv.lockis excluded by!**/*.lock
📒 Files selected for processing (93)
docs/agents/index.mdxdocs/agents/optimization.mdxpackages/nemo_evaluator_sdk/pyproject.tomlpackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/__init__.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hook_loading.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks_mcp_binding.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/enums.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_defaults.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_evaluator.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/types.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/__init__.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/metrics.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_hook_loading.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_runtime.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_mcp_run_binding_hook.pypackages/nemo_evaluator_sdk/tests/metrics/test_tunable_rag_evaluator.pypackages/nemo_platform/pyproject.tomlpackages/nemo_platform_plugin/tests/test_dispatcher.pyplugins/nemo-agents/openapi/openapi.yamlplugins/nemo-agents/pyproject.tomlplugins/nemo-agents/src/nemo_agents_plugin/cli.pyplugins/nemo-agents/src/nemo_agents_plugin/jobs/optimize_agent.pyplugins/nemo-agents/src/nemo_agents_plugin/leaderboard/render.pyplugins/nemo-agents/src/nemo_agents_plugin/service.pyplugins/nemo-agents/src/nemo_agents_plugin/utils.pyplugins/nemo-agents/tests/unit/test_cli.pyplugins/nemo-agents/tests/unit/test_improvement_jobs.pyplugins/nemo-agents/tests/unit/test_optimize_agent_job.pyplugins/nemo-agents/tests/unit/test_optimize_skills_analyze_only.pyplugins/nemo-agents/tests/unit/test_service.pyplugins/nemo-agents/tests/unit/test_utils.pyplugins/nemo-agents/tests/unit/usage/test_usage_cli.pyplugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/backend.pyplugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/gpu.pyplugins/nemo-deployments/tests/unit/backends/docker/test_gpu.pyplugins/nemo-optimization/README.mdplugins/nemo-optimization/examples/hermes-optimize/.gitignoreplugins/nemo-optimization/examples/hermes-optimize/README.mdplugins/nemo-optimization/examples/hermes-optimize/agent.yamlplugins/nemo-optimization/examples/hermes-optimize/analyzer.inference-api.yamlplugins/nemo-optimization/examples/hermes-optimize/dataset-phishing.jsonplugins/nemo-optimization/examples/hermes-optimize/dataset.jsonplugins/nemo-optimization/examples/hermes-optimize/optimize.yamlplugins/nemo-optimization/examples/hermes-optimize/package.yamlplugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-chatonly.yamlplugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-mcp.e2e.yamlplugins/nemo-optimization/pyproject.tomlplugins/nemo-optimization/src/nemo_optimization/__init__.pyplugins/nemo-optimization/src/nemo_optimization/agents.pyplugins/nemo-optimization/src/nemo_optimization/backends/__init__.pyplugins/nemo-optimization/src/nemo_optimization/backends/ga/__init__.pyplugins/nemo-optimization/src/nemo_optimization/backends/ga/backend.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/__init__.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/artifacts.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/atif_metadata.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/backend.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/config_overlay.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/early_stop.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/fabric_trial.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/search_space.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/selection.pyplugins/nemo-optimization/src/nemo_optimization/backends/optuna/study_driver.pyplugins/nemo-optimization/src/nemo_optimization/backends/protocol.pyplugins/nemo-optimization/src/nemo_optimization/config.pyplugins/nemo-optimization/src/nemo_optimization/fabric.pyplugins/nemo-optimization/src/nemo_optimization/jobs/__init__.pyplugins/nemo-optimization/src/nemo_optimization/jobs/optimize.pyplugins/nemo-optimization/src/nemo_optimization/preflight.pyplugins/nemo-optimization/src/nemo_optimization/registry.pyplugins/nemo-optimization/src/nemo_optimization/router.pyplugins/nemo-optimization/src/nemo_optimization/schemas/__init__.pyplugins/nemo-optimization/src/nemo_optimization/schemas/optimize.pyplugins/nemo-optimization/src/nemo_optimization/tasks/__init__.pyplugins/nemo-optimization/src/nemo_optimization/tasks/optimize.pyplugins/nemo-optimization/tests/conftest.pyplugins/nemo-optimization/tests/smoke_fabric_optimize_atif.pyplugins/nemo-optimization/tests/test_atif_metadata.pyplugins/nemo-optimization/tests/test_config_overlay.pyplugins/nemo-optimization/tests/test_fabric.pyplugins/nemo-optimization/tests/test_fabric_trial.pyplugins/nemo-optimization/tests/test_optimize_job.pyplugins/nemo-optimization/tests/test_router.pyplugins/nemo-optimization/tests/test_search_space.pyplugins/nemo-optimization/tests/test_selection.pyplugins/nemo-optimization/tests/test_study_driver.pypyproject.tomlservices/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/compiler.pyservices/core/models/tests/unit/controllers/backends/deployments_plugin/test_compiler.pythird_party/licenses.jsonlthird_party/osv-licenses.jsonthird_party/requirements-main.txt
💤 Files with no reviewable changes (3)
- plugins/nemo-agents/src/nemo_agents_plugin/jobs/optimize_agent.py
- plugins/nemo-agents/tests/unit/test_optimize_agent_job.py
- plugins/nemo-agents/tests/unit/test_utils.py
🚧 Files skipped from review as they are similar to previous changes (71)
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/init.py
- packages/nemo_evaluator_sdk/pyproject.toml
- plugins/nemo-agents/tests/unit/test_service.py
- pyproject.toml
- packages/nemo_platform_plugin/tests/test_dispatcher.py
- plugins/nemo-optimization/examples/hermes-optimize/dataset.json
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/init.py
- services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/compiler.py
- plugins/nemo-optimization/src/nemo_optimization/init.py
- plugins/nemo-agents/tests/unit/test_improvement_jobs.py
- plugins/nemo-agents/src/nemo_agents_plugin/leaderboard/render.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_hook_loading.py
- plugins/nemo-optimization/tests/test_search_space.py
- plugins/nemo-optimization/examples/hermes-optimize/package.yaml
- plugins/nemo-optimization/examples/hermes-optimize/analyzer.inference-api.yaml
- plugins/nemo-optimization/examples/hermes-optimize/dataset-phishing.json
- plugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-mcp.e2e.yaml
- plugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/backend.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/atif_metadata.py
- plugins/nemo-agents/src/nemo_agents_plugin/utils.py
- plugins/nemo-optimization/README.md
- plugins/nemo-optimization/examples/hermes-optimize/optimize.yaml
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/metrics.py
- third_party/licenses.jsonl
- packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_runtime.py
- plugins/nemo-optimization/examples/hermes-optimize/.gitignore
- plugins/nemo-optimization/tests/test_selection.py
- plugins/nemo-agents/src/nemo_agents_plugin/cli.py
- plugins/nemo-optimization/src/nemo_optimization/backends/init.py
- plugins/nemo-optimization/pyproject.toml
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/enums.py
- plugins/nemo-agents/src/nemo_agents_plugin/service.py
- plugins/nemo-optimization/examples/hermes-optimize/agent.yaml
- plugins/nemo-optimization/src/nemo_optimization/backends/ga/backend.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/init.py
- plugins/nemo-deployments/src/nemo_deployments_plugin/backends/docker/gpu.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks.py
- plugins/nemo-optimization/src/nemo_optimization/agents.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hook_loading.py
- plugins/nemo-optimization/src/nemo_optimization/config.py
- plugins/nemo-optimization/src/nemo_optimization/backends/ga/init.py
- plugins/nemo-optimization/src/nemo_optimization/jobs/init.py
- packages/nemo_platform/pyproject.toml
- plugins/nemo-optimization/src/nemo_optimization/tasks/init.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_defaults.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/tunable_rag_evaluator.py
- plugins/nemo-optimization/tests/test_config_overlay.py
- plugins/nemo-optimization/tests/test_atif_metadata.py
- plugins/nemo-optimization/src/nemo_optimization/schemas/init.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/types.py
- plugins/nemo-optimization/src/nemo_optimization/schemas/optimize.py
- docs/agents/optimization.mdx
- plugins/nemo-optimization/src/nemo_optimization/tasks/optimize.py
- plugins/nemo-optimization/src/nemo_optimization/backends/protocol.py
- plugins/nemo-optimization/examples/hermes-optimize/phishing.optimize.fabric-chatonly.yaml
- plugins/nemo-optimization/tests/test_fabric.py
- plugins/nemo-agents/pyproject.toml
- plugins/nemo-optimization/tests/test_optimize_job.py
- plugins/nemo-optimization/src/nemo_optimization/registry.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/early_stop.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/config_overlay.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/hooks_mcp_binding.py
- plugins/nemo-optimization/tests/test_router.py
- plugins/nemo-optimization/src/nemo_optimization/fabric.py
- plugins/nemo-optimization/src/nemo_optimization/preflight.py
- plugins/nemo-deployments/tests/unit/backends/docker/test_gpu.py
- third_party/requirements-main.txt
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/search_space.py
- plugins/nemo-optimization/src/nemo_optimization/router.py
- plugins/nemo-optimization/src/nemo_optimization/backends/optuna/study_driver.py
- plugins/nemo-optimization/src/nemo_optimization/jobs/optimize.py
🛑 Comments failed to post (1)
plugins/nemo-agents/tests/unit/usage/test_usage_cli.py (1)
204-216: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the unreadable-file test deterministic.
chmod(0o000)does not block reads by root and is not portable to Windows. The test can succeed instead of exercisingUsageParseError.Patch the parser file-read operation to raise
PermissionError. Keep the CLI assertion unchanged.🤖 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-agents/tests/unit/usage/test_usage_cli.py` around lines 204 - 216, Update test_usage_show_unreadable_result_json_exits_cleanly to mock the parser’s file-read operation so it raises PermissionError instead of relying on chmod(0o000); preserve the existing runner invocation and CLI assertions unchanged.
soluwalana
force-pushed
the
aalgo-277/solu
branch
from
August 5, 2026 19:05
e8a74b5 to
fd2398a
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Signed-off-by: Sam Oluwalana <soluwalana@nvidia.com>
Signed-off-by: Sam Oluwalana <soluwalana@nvidia.com>
Signed-off-by: Sam O <soluwalana@nvidia.com>
Signed-off-by: Sam Oluwalana <soluwalana@nvidia.com>
0.0.19 dropped MontyRepl, which breaks pydantic-ai-harness 0.3.0 and cascades into insights/auth unit test failures. Signed-off-by: Sam Oluwalana <soluwalana@nvidia.com>
Signed-off-by: Sam Oluwalana <soluwalana@nvidia.com>
Signed-off-by: Sam Oluwalana <soluwalana@nvidia.com>
soluwalana
force-pushed
the
aalgo-277/solu
branch
from
August 6, 2026 00:26
2797af8 to
653aa1e
Compare
soluwalana
enabled auto-merge
August 6, 2026 00:27
Signed-off-by: Sam Oluwalana <soluwalana@nvidia.com>
Contributor
|
/nvskills-ci |
soluwalana
force-pushed
the
aalgo-277/solu
branch
from
August 6, 2026 01:09
7aefb2a to
7e6f0c8
Compare
Contributor
|
/nvskills-ci |
Signed-off-by: Sam Oluwalana <soluwalana@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
Introduce Fabric-backed numeric hyperparameter optimization as an Agents-owned surface (
nemo agents optimize→agents.optimize), withnemo-optimizationas the shared Optuna library — not a Customizer Tune contributor (RFC Alt 5).nemo agents optimize run|submit|explain; job registration mounted by the agents plugin. Nonemo customization optimize, noOptimizationContributor.type+path), config overlays, trial selection, early stop, and artifacts (study_summary.json, CSV, Pareto plots, optional ATIFtrial_trace_map.json).FabricTrialEvaluator→AgentEvaluator+FabricAgentRuntime(no NAT in the hot path). Agent-eval/audit failures raiseStudyDriverErrorso Optuna fails that trial and continues.--agentresolve: fetch platformnemo-agents-spec-v1, translate to Fabric, merge overlaymodels(e.g. judge) from--optimize-config.nvidia.fabric.hermes). Examples underplugins/nemo-optimization/examples/hermes-optimize/(optimize-*.yamlfor--optimize-config).eval.run_hook.type: mcp_run_binding— agent checkoutagent_src+ agent-venvexecutable;mcp.servers.*.env/ top-levelargs/envstay on the package; hook rebindsurland runs create/verify/cleanup.tunable_rag_evaluatorfor judge-based scoring; Fabric task-hook loading (ref/path+attr/nemo.fabric.task_hooksentry points).nvidia-nat-config-optimizerand the legacy agents optimize job; reject NAT-shaped optimize configs.Fabric pin:
uv.sourceslocks NeMo-Fabric to55450ff…(includes FABRIC-167 MCP extensions fix). Plainuv sync --package nemo-agents-pluginis enough — no local wheel build /--no-syncworkaround required for the locked SHA.Prerequisites (Hermes / Fabric)
From the
nemo-platformrepo root. Full QA steps:plugins/nemo-optimization/examples/hermes-optimize/README.md.Run Hermes optimize (Agents CLI)
--optimize-configmust be an absolute path; dataset /base_dirpaths in the YAML are relative to process CWD (repo root).1. Chat-only smoke (
optimize-chatonly.yaml)nemo agents optimize run \ --optimize-config "$(pwd)/plugins/nemo-optimization/examples/hermes-optimize/optimize-chatonly.yaml" \ --workspace defaultExpected:
status: completed,n_trials: 2.2. Chat-only via platform
--agentUse the slim
agents/chatonly/dir only (parenthermes-optimize/includesartifacts/and fails fileset limits). Recreate withnemo agents delete hermes-optimize-chatonly -yafter editingagent.yaml.Expected:
Resolved agent 'hermes-optimize-chatonly'…,status: completed,n_trials: 2.3. MCP e2e (
optimize-mcp.yaml)Requires a phishing-analyzer agent checkout with its own venv (do not pip-install that agent into the platform venv):
Expected:
status: completed,n_trials: 4, best score near1.0when the harness calls the analyzer once.Known flake: a single dataset row with an empty Hermes response marks that sample
failedand fails the Optuna trial; if all trials fail you getno completed trials. Inspectexamples/hermes-optimize/artifacts/.fabric/hermes/runtimes/*/logs/and retry.Optional ATIF live smoke (not CI)
Point
FABRIC_QWEN_*at a reachable OpenAI-compatible endpoint (local IGW or otherwise); do not hardcode workbox hosts in configs.Test plan
pytest plugins/nemo-optimization/tests -q(ignore live smoke)packages/nemo_evaluator_sdk/tests/agent_eval/status: completed(optimize-chatonly.yaml)--agentpath → create slim agent +optimize-chatonly-via-agent.yaml→status: completedoptimize-mcp.yaml) → currently flaky on empty Hermes responses for individual dataset rows (retry / shrink dataset); previously green on this branchoptimize-*.yamlexamples, not Customizer TuneSummary by CodeRabbit
New Features
Documentation
Tests