diff --git a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/proposer.py b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/proposer.py index fbd1077ab33..0965f224ea9 100644 --- a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/proposer.py +++ b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/proposer.py @@ -1,6 +1,7 @@ # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 +import logging from pathlib import Path from typing import Any, Literal, get_args @@ -21,6 +22,8 @@ from .tools import WorkspaceTool from .util import load_framework_skills +logger = logging.getLogger(__name__) + class Improvement(BaseModel): """A proposed single-change improvement for a specific ancestor agent.""" @@ -157,41 +160,70 @@ async def run( phase=phase, max_candidates=max_candidates, ) - self._validate_improvements( + # allowed_types is every type, not just the untried ones. available_types + # still reaches the prompt, so novelty stays a *preference*: a type that + # worked in an earlier round can be used again when it is the right tool. + return self._filter_improvements( improvements=improvements, max_candidates=max_candidates, - allowed_types=set(available_types) or all_types, + allowed_types=all_types, ) - return improvements @staticmethod - def _validate_improvements( + def _filter_improvements( *, improvements: list[Improvement], max_candidates: int, allowed_types: set[str], - ) -> None: + ) -> list[Improvement]: + """Return at most ``max_candidates`` usable improvements, dropping the rest. + + An improvement is dropped when its ``optimization_type`` is outside + ``allowed_types`` or its optimization text repeats one already kept; a + repeated ``optimization_type`` is fine. Every drop is logged, so a Proposer + emitting consistently unusable output cannot look healthy. + + Raises: + ValueError: if no usable improvement remains, which leaves nothing to build. + """ if not improvements: raise ValueError("Proposer returned no improvements") - if len(improvements) > max_candidates: - raise ValueError(f"Proposer returned {len(improvements)} improvements; maximum is {max_candidates}") - seen_types: set[str] = set() + + kept: list[Improvement] = [] seen_descriptions: set[str] = set() for improvement in improvements: optimization_type = improvement.optimization_type if optimization_type not in allowed_types: - raise ValueError( - f"Proposer returned disallowed optimization_type " - f"{optimization_type!r}; allowed: {sorted(allowed_types)}" + logger.warning( + "dropping improvement with disallowed optimization_type %r; allowed: %s", + optimization_type, + sorted(allowed_types), ) - if optimization_type in seen_types: - raise ValueError(f"Proposer returned duplicate optimization_type {optimization_type!r}") - seen_types.add(optimization_type) + continue description = improvement.optimization.strip() if description in seen_descriptions: - raise ValueError(f"Proposer returned duplicate optimization text: {description!r}") + logger.warning("dropping improvement with duplicate optimization text: %r", description) + continue seen_descriptions.add(description) + kept.append(improvement) + + if not kept: + raise ValueError( + f"Proposer returned {len(improvements)} improvements, none of them usable; " + "see the warnings above for why each was dropped" + ) + + # Truncate last: a surplus improvement past the cut may be the only usable + # one, so the cut has to fall on the kept list rather than the raw one. + if len(kept) > max_candidates: + logger.warning( + "Proposer returned %d usable improvements; keeping the first %d", + len(kept), + max_candidates, + ) + kept = kept[:max_candidates] + return kept @strategy(CodeActStrategy(config=CodeActConfig(max_iterations=20, cell_timeout=3600.0))) async def _run_with_context( @@ -210,8 +242,8 @@ async def _run_with_context( Args: - analysis (str): round analysis with root causes already enumerated - evolution_history (str): markdown table of prior rounds, for context - - tried_types (list[str]): optimization_types already attempted — AVOID these - - available_types (list[str]): types not yet tried — PICK FROM THESE + - tried_types (list[str]): optimization_types already attempted + - available_types (list[str]): types not yet tried — PREFER THESE - survivors (list[dict]): each {id, reward, trajectory_reward, metadata, architecture}; these are your branching candidates with their architecture.md already loaded @@ -238,7 +270,7 @@ async def _run_with_context( independently of the fix, drop the hypothesis. 2. Pick the ancestor from `survivors` most affected by that root cause. 3. Load the matching card with `doc(self.optimize.)` and pick ONE - optimization_type from `available_types` that the card covers. + optimization_type that the card covers, preferring `available_types`. 4. Read the ancestor's architecture diagram (in `survivors[*].architecture`) to understand the current graph shape. If the change touches a skill, find it in the diagram — do NOT open source files. @@ -262,13 +294,15 @@ async def _run_with_context( ## Phase - exploration: novel directions, even speculative ones - exploitation: refine the current best survivor. When the obvious targeted - improvements have already been tried, do NOT stop — pick the best untried - direction from `available_types` and explore it. + improvements have already been tried, do NOT stop — take the best remaining + direction, an untried type when one fits and a tried one when it does not. ## MANDATORY - Return between 1 and max_candidates Improvements. NEVER return []. - - Each Improvement.optimization_type MUST be in `available_types`. If - `available_types` is empty, pick the least-tried type from the tried set. + - Each Improvement.optimization_type MUST be in `available_types` or + `tried_types`. Prefer `available_types`; reuse a tried type when it is + the tool that actually addresses the root cause. A type that worked in + an earlier round is not spent. - Each improvement in a round should target a different degree of freedom; two improvements touching the same axis are only allowed when no other viable direction exists. diff --git a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/trace_scorer.py b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/trace_scorer.py index 58360492f25..4ddc8908d77 100644 --- a/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/trace_scorer.py +++ b/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/trace_scorer.py @@ -26,7 +26,11 @@ class GroupLeafScore(BaseModel): reason: str span_ids: list[str] = Field( default_factory=list, - description="Span IDs from the trace that contain the key evidence cited in `reason`.", + description=( + "Span IDs from the trace that contain the key evidence cited in `reason`. " + "Empty when the trace has no turns to look them up from, which is the normal " + "case for an agent that makes no LLM calls." + ), ) @@ -121,6 +125,14 @@ async def run( or constructed from the overview text. Include only spans that directly support the score — not every span. + A trace with no turns is expected, not a failure. Both lookups above are indexed by + turn, so an agent that makes no LLM calls — deterministic, rule-based, or purely + tool-driven — has nothing for them to return, and they yield `None` however many times + you call them. When `get_overview()` reports `Turns: 0`, leave `span_ids` empty and + ground the reason in the call graph instead: name the methods that ran, the order they + ran in, and their status. That is sufficient evidence for such a trace. Do not retry the + turn lookups, and do not put the abbreviated ids from the overview text into `span_ids`. + Returns: dict[str, GroupLeafScore]: scores indexed by agent ID; each entry includes a numeric score, a reason citing specific trace evidence, and the span IDs that ground the reason. diff --git a/plugins/nemo-experimentalist/tests/experimentalist/test_proposer_contract.py b/plugins/nemo-experimentalist/tests/experimentalist/test_proposer_contract.py new file mode 100644 index 00000000000..74bbdb910ef --- /dev/null +++ b/plugins/nemo-experimentalist/tests/experimentalist/test_proposer_contract.py @@ -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 diff --git a/plugins/nemo-experimentalist/tests/experimentalist/test_proposer_validation.py b/plugins/nemo-experimentalist/tests/experimentalist/test_proposer_validation.py new file mode 100644 index 00000000000..20a1ef24796 --- /dev/null +++ b/plugins/nemo-experimentalist/tests/experimentalist/test_proposer_validation.py @@ -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")]) diff --git a/plugins/nemo-experimentalist/tests/experimentalist/test_trace_scorer_contract.py b/plugins/nemo-experimentalist/tests/experimentalist/test_trace_scorer_contract.py new file mode 100644 index 00000000000..ca90cf75935 --- /dev/null +++ b/plugins/nemo-experimentalist/tests/experimentalist/test_trace_scorer_contract.py @@ -0,0 +1,62 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Pin the parts of the scorer's contract that a turn-less trace depends on. + +`GroupLeafScorer.run` is a CodeAct prompt, so its docstring is the instruction the +model follows. The only sanctioned evidence lookups are indexed by turn, so a trace +with zero turns needs the contract to name that case, permit an empty `span_ids`, +and point at evidence the scorer can actually reach. These tests keep that escape +hatch present and keep the schema able to represent it. +""" + +from __future__ import annotations + +import inspect + +from nemo_experimentalist_plugin.experimentalist.components.trace_scorer import ( + GroupLeafScore, + GroupLeafScorer, +) + + +def _run_contract() -> str: + """The prompt with whitespace collapsed, so assertions do not hinge on where it wraps.""" + doc = inspect.getdoc(GroupLeafScorer.run) + assert doc is not None, "GroupLeafScorer.run must keep its docstring; it is the prompt" + return " ".join(doc.lower().split()) + + +def test_contract_tells_the_scorer_what_to_do_with_no_turns() -> None: + """Without this, the only permitted evidence source is unreachable and the run dies.""" + contract = _run_contract() + assert "turns: 0" in contract, "the contract must name the condition the scorer can observe" + assert "no turns" in contract + assert "leave `span_ids` empty" in contract, "the contract must say what to return instead" + + +def test_contract_offers_an_alternative_grounding() -> None: + """Permission to return nothing is not enough; the reason must still be evidence-backed.""" + contract = _run_contract() + assert "call graph" in contract, "a turn-less trace still carries a call graph to cite" + for required in ("methods that ran", "order they ran in", "status"): + assert required in contract, f"the contract must name {required!r} as evidence to cite" + + +def test_contract_still_forbids_inventing_span_ids() -> None: + """Relaxing the requirement must not license guessed or abbreviated IDs.""" + contract = _run_contract() + for forbidden in ("never abbreviated, guessed", "do not put the abbreviated ids"): + assert forbidden in contract, f"the contract must still refuse {forbidden!r}" + + +def test_score_is_valid_without_span_ids() -> None: + """The schema has to accept what the contract now asks for in the turn-less case.""" + score = GroupLeafScore(score=0.6, reason="handle_lookup ran and returned OK; solve dispatched to it") + assert score.span_ids == [] + + +def test_score_still_accepts_span_ids() -> None: + """The turn-less path must not become the only path.""" + score = GroupLeafScore(score=0.9, reason="cited", span_ids=["abc123"]) + assert score.span_ids == ["abc123"]