Repository navigation
fix(experimentalist): stop discarding usable work in the optimization loop #1163
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
c2859f6
fix(experimentalist): salvage imperfect proposals instead of aborting…
schuellc-nvidia ba8b4a3
fix(experimentalist): let trajectory scoring survive a trace with no …
schuellc-nvidia b4959e3
fix(experimentalist): cut the surplus after filtering, not before
schuellc-nvidia aa5c0ab
fix(experimentalist): let the prompt propose a type it already used
schuellc-nvidia File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
43 changes: 43 additions & 0 deletions
43
plugins/nemo-experimentalist/tests/experimentalist/test_proposer_contract.py
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| """Pin the half of type reuse that lives in the prompt rather than the validator. | ||
|
|
||
| `Proposer._run_with_context` is a CodeAct prompt, so its docstring is the | ||
| instruction the model follows. `_filter_improvements` accepting a repeated | ||
| `optimization_type` only stops the run from dying on one; if the prompt still | ||
| requires every type to be untried, the model never proposes one and the | ||
| allowance is unreachable. These tests keep the two halves in agreement. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import inspect | ||
|
|
||
| from nemo_experimentalist_plugin.experimentalist.components.proposer import Proposer | ||
|
|
||
|
|
||
| def _proposal_contract() -> str: | ||
| """The prompt with whitespace collapsed, so assertions do not hinge on where it wraps.""" | ||
| doc = inspect.getdoc(Proposer._run_with_context) | ||
| assert doc is not None, "Proposer._run_with_context must keep its docstring; it is the prompt" | ||
| return " ".join(doc.lower().split()) | ||
|
|
||
|
|
||
| def test_contract_permits_reusing_a_tried_type() -> None: | ||
| """A hard `MUST be in available_types` makes reuse unreachable while any type is untried.""" | ||
| contract = _proposal_contract() | ||
| assert "must be in `available_types` or" in contract, "the contract must not restrict the pick to untried types" | ||
| assert "`tried_types`" in contract, "reuse is only reachable if tried types are a legal pick" | ||
|
|
||
|
|
||
| def test_contract_keeps_novelty_as_a_preference() -> None: | ||
| """Reuse being legal must not make it the default; untried directions are still preferred.""" | ||
| contract = _proposal_contract() | ||
| assert "prefer" in contract, "novelty has to survive as a preference, not vanish with the restriction" | ||
|
|
||
|
|
||
| def test_contract_does_not_restrict_the_card_pick_to_untried_types() -> None: | ||
| """The per-improvement step would otherwise contradict the MANDATORY block.""" | ||
| contract = _proposal_contract() | ||
| assert "pick one optimization_type from `available_types`" not in contract |
135 changes: 135 additions & 0 deletions
135
plugins/nemo-experimentalist/tests/experimentalist/test_proposer_validation.py
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| """Pin what `Proposer._filter_improvements` keeps, drops, and refuses. | ||
|
|
||
| Imperfect output is salvaged: an unusable improvement is dropped on its own and | ||
| the round continues with fewer candidates. Only an empty result is fatal. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import logging | ||
|
|
||
| import pytest | ||
| from nemo_experimentalist_plugin.experimentalist.components.proposer import Improvement, Proposer | ||
|
|
||
| ALLOWED = {"edit_concrete_method", "edit_config", "add_concrete_method"} | ||
|
|
||
|
|
||
| def _improvement(optimization_type: str, optimization: str, ancestor: str = "agent-0") -> Improvement: | ||
| return Improvement( | ||
| ancestor=ancestor, | ||
| root_cause="the agent underperforms because a handler is missing", | ||
| optimization=optimization, | ||
| optimization_type=optimization_type, | ||
| ) | ||
|
|
||
|
|
||
| def _filter(improvements: list[Improvement], max_candidates: int = 3) -> list[Improvement]: | ||
| return Proposer._filter_improvements( | ||
| improvements=improvements, | ||
| max_candidates=max_candidates, | ||
| allowed_types=ALLOWED, | ||
| ) | ||
|
|
||
|
|
||
| def test_repeated_optimization_type_is_kept() -> None: | ||
| """Two candidates may legitimately need the same kind of edit in different places.""" | ||
| kept = _filter( | ||
| [ | ||
| _improvement("edit_concrete_method", "widen the name pattern"), | ||
| _improvement("edit_concrete_method", "reorder the dispatch chain"), | ||
| ] | ||
| ) | ||
| assert len(kept) == 2 | ||
|
|
||
|
|
||
| def test_every_candidate_may_share_one_type() -> None: | ||
| """The degenerate case: late rounds often have only one kind of work left.""" | ||
| kept = _filter( | ||
| [ | ||
| _improvement("edit_config", "raise the input limit"), | ||
| _improvement("edit_config", "raise the retry budget"), | ||
| _improvement("edit_config", "raise the timeout"), | ||
| ] | ||
| ) | ||
| assert len(kept) == 3 | ||
|
|
||
|
|
||
| def test_surplus_improvements_are_truncated() -> None: | ||
| """Keeping the first N assumes the Proposer ordered them by priority.""" | ||
| kept = _filter( | ||
| [ | ||
| _improvement("edit_config", "first"), | ||
| _improvement("edit_concrete_method", "second"), | ||
| _improvement("add_concrete_method", "third"), | ||
| ], | ||
| max_candidates=2, | ||
| ) | ||
| assert [i.optimization for i in kept] == ["first", "second"] | ||
|
|
||
|
|
||
| def test_surplus_is_counted_after_the_unusable_are_dropped() -> None: | ||
| """Cutting first would discard the only usable improvement and make the round fatal.""" | ||
| kept = _filter( | ||
| [ | ||
| _improvement("add_subagent", "delegate to a new agent"), | ||
| _improvement("add_subagent", "delegate to another new agent"), | ||
| _improvement("edit_config", "raise the input limit"), | ||
| ], | ||
| max_candidates=2, | ||
| ) | ||
| assert [i.optimization for i in kept] == ["raise the input limit"] | ||
|
|
||
|
|
||
| def test_type_outside_the_allowed_set_is_dropped() -> None: | ||
| """A valid OptimizationType the caller did not permit for this run. | ||
|
|
||
| `add_subagent` is a real member of the literal -- an invented string is | ||
| rejected by Pydantic at construction and never reaches this filter. | ||
| """ | ||
| kept = _filter( | ||
| [ | ||
| _improvement("edit_config", "raise the input limit"), | ||
| _improvement("add_subagent", "delegate to a new agent"), | ||
| ] | ||
| ) | ||
| assert [i.optimization for i in kept] == ["raise the input limit"] | ||
|
|
||
|
|
||
| def test_duplicate_optimization_text_is_dropped() -> None: | ||
| """Identical proposals are genuinely redundant; the first one survives.""" | ||
| kept = _filter( | ||
| [ | ||
| _improvement("edit_config", "raise the input limit"), | ||
| _improvement("edit_concrete_method", "raise the input limit"), | ||
| _improvement("add_concrete_method", "add a totals handler"), | ||
| ] | ||
| ) | ||
| assert [i.optimization for i in kept] == ["raise the input limit", "add a totals handler"] | ||
|
|
||
|
|
||
| def test_drops_are_logged(caplog: pytest.LogCaptureFixture) -> None: | ||
| """Silent degradation would let a consistently bad Proposer look healthy.""" | ||
| with caplog.at_level(logging.WARNING): | ||
| _filter( | ||
| [ | ||
| _improvement("edit_config", "raise the input limit"), | ||
| _improvement("add_subagent", "delegate to a new agent"), | ||
| _improvement("edit_concrete_method", "raise the input limit"), | ||
| ] | ||
| ) | ||
| assert "disallowed optimization_type" in caplog.text | ||
| assert "duplicate optimization text" in caplog.text | ||
|
|
||
|
|
||
| def test_empty_input_is_fatal() -> None: | ||
| with pytest.raises(ValueError, match="no improvements"): | ||
| _filter([]) | ||
|
|
||
|
|
||
| def test_everything_dropped_is_fatal() -> None: | ||
| """Nothing usable means nothing to build, so the round cannot continue.""" | ||
| with pytest.raises(ValueError, match="none of them usable"): | ||
| _filter([_improvement("add_subagent", "delegate to a new agent")]) |
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.