Align the finance agent loop and finance_sec_search resource server with the vals-ai - #2055
Merged
Merged
Conversation
…nly responses Pre-change, the agent loop terminated as soon as the model returned a message-only step (no function_call), which silently truncated rollouts whenever the model emitted explanatory text between tool calls. This diverged from vals-ai/finance-agent eval, whose _before_query appends a "Continue." TextInput when the last item is a RawResponse and _should_stop is False, so the loop keeps running until either submit_final_result fires or max_iterations is reached. The training-side effect was that Nemotron-Nano's rs0 chunk recorded many "completed" rollouts that were actually stopped mid-trajectory, and the policy could not learn the multi-step retrieval pattern eval expects. Known pre-existing limitation (NOT introduced here): if a model returns only reasoning items (no function_call and no message), the loop makes no progress and only terminates via max_steps. Both branches in the text-only check are empty for that case. Today operators bound this via max_steps (e.g. 50 in nvflow's finance_sec_search_env.yaml). Operational note for downstream: max_steps becomes load-bearing under this change because every text response now consumes an extra step. The rollout-length distribution will shift up vs the pre-change behavior -- monitor max_steps_hit_rate after rolling this out and re-tune if needed. Tests: - new TestResponses::test_text_only_response_injects_continue_until_max_steps - new TestResponses::test_continue_injection_stops_at_submit_final_result - renamed test_tool_call_then_text_terminates -> test_tool_call_then_text_continues_until_max_steps with updated assertions - updated test_context_overflow_with_truncation to terminate via done-tool - updated test_string_input_converted_to_message with max_steps=1 Signed-off-by: Pritam Gundecha <pgundecha@nvidia.com>
…utput diagnostic retrieve_information used to swallow 4xx/5xx responses from the vLLM retrieval server and report the vague "Retrieval LLM returned no output" message, because the JSON parser short-circuited on the error body. Operators could not tell whether the retrieval model was misconfigured, oversubscribed, or quietly silent on a valid request. Two changes: 1. Check llm_response.ok before parsing. On 4xx/5xx, return an error with HTTP status + body excerpt (capped at 500 chars so vLLM HTML error pages cannot balloon agent context). 2. In the empty-output branch, surface any incomplete_details.reason (e.g. max_output_tokens, content_filter) plus top-level status / error from the parsed response, so the agent can see why output was empty instead of guessing. Both diagnostics are appended to the tool result string, so the policy sees them in the next-step context window and can adapt (e.g. retry with a smaller prompt on max_output_tokens). Tests: - new test_http_error_surfaces_status_and_body - new test_http_error_body_is_capped_at_500_chars - new test_empty_output_includes_incomplete_details Signed-off-by: Pritam Gundecha <pgundecha@nvidia.com>
…eve_information
The retrieve_information `prompt` field description had been stripped to a
one-line note ("Prompt with {{key_name}} placeholders"), dropping the worked
example the upstream vals-ai finance-agent benchmark ships. Models omit the
required {{key_name}} placeholder on ~330/seed retrieval calls (tool-format
violations) in the nano smoke.
Restore a description with an explicit example and the format requirement,
matching the reference benchmark's retrieve_information contract.
Signed-off-by: Pritam Gundecha <pgundecha@nvidia.com>
…window/HTTP errors and make retrieval_max_output_tokens Optional (#1540) disable on-disk cache by default for eval, surface retrieval context-window/HTTP errors and make retrieval_max_output_tokens Optional - add use_cache flag (default False) gating all disk reads/writes - make retrieval_max_output_tokens Optional (None = uncapped for eval) - surface retrieval LLM HTTP errors + empty-output incomplete_details diagnostic - restore {{key_name}} worked example in retrieve_information prompt description --------- Signed-off-by: Ushnish De <ude@nvidia.com>
…ults
Wrap tavily_api_key and search_judge_model_* interpolations in
${oc.select:<key>,<default>} so the env resolves standalone under
`gym env start` / `+dry_run` without every key being supplied (bare
${key} raises "unsupported interpolation" otherwise). NVFlow overlays
and env.yaml still override by setting the referenced keys. The null
tavily default keeps web search disabled unless a key is provided.
Signed-off-by: Pritam Gundecha <pgundecha@nvidia.com>
bxyu-nvidia
approved these changes
Jul 16, 2026
OlegSudakov
pushed a commit
to OlegSudakov/Gym
that referenced
this pull request
Aug 7, 2026
…ith the vals-ai (NVIDIA-NeMo#2055) Changes vs main - finance_agent loop parity — On text-only model responses (no tool calls), injects a "Continue." user message and keeps looping until submit_final_result or max_steps, instead of stopping after one step. Matches vals-ai get_agent.py behavior and avoids rollouts ending early when the model emits explanatory text between tool calls. - Fresh filings for eval — Adds use_cache (default false) to finance_sec_search; when off, all on-disk cache reads/writes are skipped so every request fetches live SEC data. README documents eval vs training usage. - Uncapped retrieval tokens for eval — retrieval_max_output_tokens is now Optional[int] (None = no cap, inherits full generation budget; set an int to cap for training). - Better retrieval error diagnostics — retrieve_information surfaces HTTP 4xx/5xx with status + body excerpt (≤500 chars), and empty-output cases include incomplete_details.reason (e.g. max_output_tokens) instead of a generic “no output” message. - Tool description fix — Restores the explicit {{key_name}} placeholder example in the retrieve_information prompt field description to cut down tool-format violations. - Standalone config resolution — Wraps tavily_api_key and search_judge_model_* in ${oc.select:...} so gym env start / +dry_run works without supplying every key; NVFlow overlays still override. - Tests — New/updated unit tests for Continue-injection loop behavior, HTTP error surfacing, empty-output diagnostics, and cache gating (~270 lines of new test coverage in finance_sec_search). --------- Signed-off-by: Pritam Gundecha <pgundecha@nvidia.com> Signed-off-by: Ushnish De <ude@nvidia.com> Co-authored-by: Pritam Gundecha <pgundecha@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
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.
Changes vs main