[krishnpci]

What does this PR do?

Resolves #2880

Adds resources_servers/indian_banking, a multi-turn customer-support environment in which the policy staffs the chat desk of a synthetic India-based retail bank: the agent talks to an LLM-simulated customer, operates 34 tools (33 servicing tools plus human hand-off) against a deterministic in-process banking engine, consults a written policy and a 59-article keyword knowledge base, and is scored on the tool calls it made, the database it left behind, and what it told the customer.

Public tau-style environments cover airline, retail and telecom; retail banking has a different tool surface (deposits, loans, cards, mandates, cheques, servicing requests, hand-off) and, in the Indian setting, its own rules (KYC/re-KYC gating, INR rate cards and EMIs, government schemes, TDS). The reward is built for training as well as evaluation: a strict gate for benchmarking and a bounded dense signal with an LLM-judged NL-assertion component that gives GRPO-style training a gradient without letting partial credit reach a pass.

Ships fully in-tree — train.jsonl (250 rows), validation.jsonl (50), db.json (197 synthetic customers), kb.json (59 articles), policy, agent instruction, 5 example rows / metrics / rollouts — no external download. app.py is a GymnasiumServer with per-episode database isolation under a frozen clock and ID seed, contextvars-bound so concurrent rollouts stay isolated. 21 offline tests. No changes to NeMo Gym core; runtime dependency nemo_gym only. Two small evaluator utilities derive from tau2-bench (MIT, attributed in-file and in ATTRIBUTIONS.md).

Verification logic

Each task's reward_basis selects the checks:

Check	Passes when
ACTION	every gold call made in order (SEQ_STRICT = True, documented) with matching arguments; no unrequested write; an errored call counts only if the gold action itself errors on replay
DB	final database hash equals a gold-action replay after state normalisation (end-state check)
COMMUNICATE	every required fact told to the customer; no internal IDs or tool names leaked
NL_ASSERTION	LLM judge over the customer-facing transcript only; never in the strict gate; fails open

strict = AND of the active binary checks; reward = 1 − efficiency_cost on a pass, else strict + 0.4·dense, so partial credit stays below the smallest pass increment. Every one of the 300 tasks replays its gold actions at strict = 1.0.

Review feedback addressed (second commit)

Problem: errored tool calls could satisfy gold actions and never counted as wrong writes; initialization_actions leaked into the ACTION/purity checks; lenient-mode mixed turns dropped their text from the customer-facing history; the judge cache was unbounded and ignored the judge model; the splits needed an external download.

Change: gold-error-aware ACTION matching; world state cleared after init (failures raise); mixed-turn text kept in the history; bounded LRU judge cache keyed by judge model; numeric argument coercion; dataset moved in-tree; one regression test per fix.

Validation

• pytest resources_servers/indian_banking/tests — 21 passed; gym env test --resources-server indian_banking passes; pre-commit run --all-files clean.
• Gold-action replay 300/300 strict; example_metrics.json reports 5 examples; example_rollouts.jsonl holds exactly 5 rollouts (rewards 0.15–1.0).
• Live end-to-end rollouts and a 2-step GRPO smoke (rewards 0.38 → 0.42) against a served policy + user-sim/judge.
• Branch rebased on current main (b8250f3d); both commits DCO-signed.

Notes for reviewers

• The user simulator and judge need one OpenAI-compatible endpoint (set in env.yaml); the judge must be a separate model server from the policy.
• The top-level README.md change is generated by the update-readme-table pre-commit hook, not hand-edited.
• All data is synthetic: invented organisation names checked against real companies, placeholder contact details, invented street-level addresses.
• verified: false as injected; awaits maintainer baselining.

Checklist

• I have read the contributing guidelines.
• The change is focused; unrelated "drive-by" edits are tracked as separate issues/PRs.
• Tests added or updated and pass locally, or N/A for docs-only / non-code changes (so CI unit/server checks pass when applicable).
• Pre-commit checks pass locally (pre-commit run --all-files) (so CI lint/format/copyright pass).
• All commits have DCO sign-off (git commit -s) (so the DCO check passes).

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[nimkothari]

@krishnpci based on code agent review. Following is the finding:

Thanks for this contribution — the determinism machinery (frozen clock, content-hashed ids, per-episode pickle clones bound via contextvars) and the judge design (fail-open, transcript-only, never in strict, bounded share) are genuinely well thought out, and attribution hygiene is exemplary. I cloned the branch at 6c79372b, ran the suite myself (16/16 pass, fully offline), and AST-cross-checked all 34 tool schemas against function signatures (exact match).

That said, I found a few blockers and three correctness bugs in the reward path (two verified by executing the code), so I'd like to see another revision before merge.

Blockers

B1. The dataset repository is not accessible — config admits it's a placeholder

https://huggingface.co/datasets/NPCI/nemo-gym-indian-banking returns 401 (private or not yet uploaded). The config comment says it directly: "The dataset repository (placeholder below until the upload lands)".

Consequences as merged:

• gym dataset collate --download fails; all 300 tasks, db.json, and kb.json are unobtainable, so the environment cannot actually be served from this branch.
• The headline claims ("gold-action replay 300/300 strict," the GRPO run) are unverifiable by reviewers.
• The regenerated root README.md environment table links to the dead repo.

Please publish the repo (or ship the data in-tree) and rerun the download path end-to-end before merge.

B2. SEQ_STRICT = True in code; the PR body and the README say default False

core/reward.py:44 sets SEQ_STRICT = True, while the PR body ("An optional SEQ_STRICT constant (off by default, matching tau2-bench's set-based check)") and resources_servers/indian_banking/README.md ("SEQ_STRICT (default False)… state which setting was used") claim the opposite. This is load-bearing, not cosmetic — verified with the test fixture DB:

# gold = [get_account_balance, toggle_card_freeze]; agent calls them reversed
strict=0.0  seq_frac=0.50  action_frac=1.00  score=0.360

An agent making exactly the gold calls in a different order fails strict. So as shipped, scores are not tau2-comparable, contradicting the "Parity with the tau2-bench decomposition" section. Please either flip the default to False or restate the claims — and, either way, note which setting produced the reported 300/300 gold replay and the RL numbers.

B3. Contradictory statements about where db.json / kb.json live

• PR body: "db.json (5.4 MB) ships in-tree for zero-setup tests"
• data/.gitignore: excludes both files
• configs/indian_banking.yaml: "download them … and place them in data/ before serving"
• Environment README.md: says both things in different paragraphs ("place db.json and kb.json in data/ before serving" vs. "ships in-tree along with data/db.json, data/kb.json")

(The offline tests are fine — they build tiny fixtures, which is the right pattern.) Please pick one true story and make all four places agree.

Correctness bugs (verified by execution)

C1. Errored tool calls satisfy the ACTION check

engine.apply_tool records calls with error: true, but _action_reward's matches() never looks at it. Repro (tiny fixture DB, ₹50k balance):

gold = [create_fd(principal=1,000,000, …)]
agent calls create_fd with the same args -> tool returns INSUFFICIENT_FUNDS, DB unchanged
reward_basis = ["ACTION"]  ->  strict=1.0, score=1.000

tau2's set-based action check has the same convention, and the DB check catches it whenever DB is in the basis — but for ACTION-only tasks with write golds this is a clean false-pass / reward-hacking vector. Recommend excluding error calls from matches() (at minimum for write golds), or documenting it as accepted semantics with the DB check as the mitigant.

C2. initialization_actions pollute world["calls"] — any task using them becomes unpassable (latent)

app.reset() executes initial-state actions via engine.apply_tool, which appends them to world["calls"]. _action_reward then counts any init write outside the gold set as a bad_write:

reset() runs init action toggle_card_freeze; agent makes the exact gold call
-> strict=0.0, bad_writes=1.0 (task can never pass)

All in-tree rows have initialization_actions: null, so it's latent today, but the first task that uses init writes silently can't score strict=1 — and the failure will be hard to trace. Suggest executing init actions without recording (or world["calls"].clear() / reset transferred right after init). Related: init-action errors are silently ignored — please surface them rather than seeding a broken world.

C3. Lenient-mode mixed turns silently drop assistant text

In app.step(), a text+tool-call turn (with strict_turn_protocol: false) executes the tools but the text never enters nl_history. The user simulator answers without seeing it, the COMMUNICATE check can't credit it, and the judge never sees it. The instruction tells the policy never to mix, but when it happens the evaluator loses signal with no trace. Consider appending the text to nl_history in lenient mode (the sim/judge tolerate it fine), or at minimum emitting a dropped_text_turns counter in info so trainers can measure the loss.

Smaller items

• judge._cache is unbounded and keyed without judge-model identity — it grows forever on long RL runs and returns stale verdicts if the judge model changes in-process. Include the model name in cache_key and bound the cache.
• README tools section lists request_cheque_book / request_duplicate_statement among "the only calls that can count as wrong writes" — both are in SOFT_WRITE_TOOLS, excluded from bad_writes. Docs should match.
• args_match is type-strict ("50000" vs 50000 false-fails). Dense partially absorbs it, but a numeric-coercion pass would reduce false failures for policies that emit string amounts.
• _FORCED_TOOL_ARGS forces top_k=1 on search_knowledge_base while the schema advertises top_k and the policy encourages re-querying — worth one line in agent_instruction.txt so the policy isn't penalized for asking.
• Nit: PR body says example rollout rewards span "0.05–1.0"; actual range in example_rollouts.jsonl is 0.373–1.0.

Unverifiable-until-data-lands claims

300/300 gold replay, 50 scenario families, 197-customer DB, and the training/validation reward distributions — happy to re-check these once B1 is resolved.

Suggested merge conditions

1. B1: publish the dataset (or ship data in-tree); rerun gym dataset collate --download from a clean clone.
2. B2: resolve the SEQ_STRICT contradiction; annotate which setting produced every reported number.
3. C1 + C2 fixed (C2 is a ~2-line change; C1 deserves an explicit tau2-parity decision).
4. B3 + README/write-list docs aligned with the code.

Happy to re-review quickly once these land — the core design (in-process engine, gold-replay DB check, bounded judge) is exactly the right shape for this class of environment.

[krishnpci]

@krishnpci based on code agent review. Following is the finding:

Thanks for this contribution — the determinism machinery (frozen clock, content-hashed ids, per-episode pickle clones bound via contextvars) and the judge design (fail-open, transcript-only, never in strict, bounded share) are genuinely well thought out, and attribution hygiene is exemplary. I cloned the branch at 6c79372b, ran the suite myself (16/16 pass, fully offline), and AST-cross-checked all 34 tool schemas against function signatures (exact match).

That said, I found a few blockers and three correctness bugs in the reward path (two verified by executing the code), so I'd like to see another revision before merge.

Blockers

B1. The dataset repository is not accessible — config admits it's a placeholder

https://huggingface.co/datasets/NPCI/nemo-gym-indian-banking returns 401 (private or not yet uploaded). The config comment says it directly: "The dataset repository (placeholder below until the upload lands)".

Consequences as merged:

• gym dataset collate --download fails; all 300 tasks, db.json, and kb.json are unobtainable, so the environment cannot actually be served from this branch.
• The headline claims ("gold-action replay 300/300 strict," the GRPO run) are unverifiable by reviewers.
• The regenerated root README.md environment table links to the dead repo.

Please publish the repo (or ship the data in-tree) and rerun the download path end-to-end before merge.

B2. SEQ_STRICT = True in code; the PR body and the README say default False

core/reward.py:44 sets SEQ_STRICT = True, while the PR body ("An optional SEQ_STRICT constant (off by default, matching tau2-bench's set-based check)") and resources_servers/indian_banking/README.md ("SEQ_STRICT (default False)… state which setting was used") claim the opposite. This is load-bearing, not cosmetic — verified with the test fixture DB:

# gold = [get_account_balance, toggle_card_freeze]; agent calls them reversed
strict=0.0  seq_frac=0.50  action_frac=1.00  score=0.360

An agent making exactly the gold calls in a different order fails strict. So as shipped, scores are not tau2-comparable, contradicting the "Parity with the tau2-bench decomposition" section. Please either flip the default to False or restate the claims — and, either way, note which setting produced the reported 300/300 gold replay and the RL numbers.

B3. Contradictory statements about where db.json / kb.json live

• PR body: "db.json (5.4 MB) ships in-tree for zero-setup tests"
• data/.gitignore: excludes both files
• configs/indian_banking.yaml: "download them … and place them in data/ before serving"
• Environment README.md: says both things in different paragraphs ("place db.json and kb.json in data/ before serving" vs. "ships in-tree along with data/db.json, data/kb.json")

(The offline tests are fine — they build tiny fixtures, which is the right pattern.) Please pick one true story and make all four places agree.

Correctness bugs (verified by execution)

C1. Errored tool calls satisfy the ACTION check

engine.apply_tool records calls with error: true, but _action_reward's matches() never looks at it. Repro (tiny fixture DB, ₹50k balance):

gold = [create_fd(principal=1,000,000, …)]
agent calls create_fd with the same args -> tool returns INSUFFICIENT_FUNDS, DB unchanged
reward_basis = ["ACTION"]  ->  strict=1.0, score=1.000

tau2's set-based action check has the same convention, and the DB check catches it whenever DB is in the basis — but for ACTION-only tasks with write golds this is a clean false-pass / reward-hacking vector. Recommend excluding error calls from matches() (at minimum for write golds), or documenting it as accepted semantics with the DB check as the mitigant.

C2. initialization_actions pollute world["calls"] — any task using them becomes unpassable (latent)

app.reset() executes initial-state actions via engine.apply_tool, which appends them to world["calls"]. _action_reward then counts any init write outside the gold set as a bad_write:

reset() runs init action toggle_card_freeze; agent makes the exact gold call
-> strict=0.0, bad_writes=1.0 (task can never pass)

All in-tree rows have initialization_actions: null, so it's latent today, but the first task that uses init writes silently can't score strict=1 — and the failure will be hard to trace. Suggest executing init actions without recording (or world["calls"].clear() / reset transferred right after init). Related: init-action errors are silently ignored — please surface them rather than seeding a broken world.

C3. Lenient-mode mixed turns silently drop assistant text

In app.step(), a text+tool-call turn (with strict_turn_protocol: false) executes the tools but the text never enters nl_history. The user simulator answers without seeing it, the COMMUNICATE check can't credit it, and the judge never sees it. The instruction tells the policy never to mix, but when it happens the evaluator loses signal with no trace. Consider appending the text to nl_history in lenient mode (the sim/judge tolerate it fine), or at minimum emitting a dropped_text_turns counter in info so trainers can measure the loss.

Smaller items

• judge._cache is unbounded and keyed without judge-model identity — it grows forever on long RL runs and returns stale verdicts if the judge model changes in-process. Include the model name in cache_key and bound the cache.
• README tools section lists request_cheque_book / request_duplicate_statement among "the only calls that can count as wrong writes" — both are in SOFT_WRITE_TOOLS, excluded from bad_writes. Docs should match.
• args_match is type-strict ("50000" vs 50000 false-fails). Dense partially absorbs it, but a numeric-coercion pass would reduce false failures for policies that emit string amounts.
• _FORCED_TOOL_ARGS forces top_k=1 on search_knowledge_base while the schema advertises top_k and the policy encourages re-querying — worth one line in agent_instruction.txt so the policy isn't penalized for asking.
• Nit: PR body says example rollout rewards span "0.05–1.0"; actual range in example_rollouts.jsonl is 0.373–1.0.

Unverifiable-until-data-lands claims

300/300 gold replay, 50 scenario families, 197-customer DB, and the training/validation reward distributions — happy to re-check these once B1 is resolved.

Suggested merge conditions

1. B1: publish the dataset (or ship data in-tree); rerun gym dataset collate --download from a clean clone.
2. B2: resolve the SEQ_STRICT contradiction; annotate which setting produced every reported number.
3. C1 + C2 fixed (C2 is a ~2-line change; C1 deserves an explicit tau2-parity decision).
4. B3 + README/write-list docs aligned with the code.

Happy to re-review quickly once these land — the core design (in-process engine, gold-replay DB check, bounded judge) is exactly the right shape for this class of environment.

@nimkothari

Thank you for the thorough review - the executed repros were spot-on, and all three
correctness findings were real. Everything below is addressed in one revision commit
(Address review: reward correctness, protocol leniency, in-tree dataset), with a
regression test per finding (suite is now 21 tests).

Changes

B1 (dataset access). The dataset now ships fully in-tree under data/:
train.jsonl (250), validation.jsonl (50), db.json, kb.json, alongside the
already-committed example files. The source: blocks and the placeholder repo id are
removed from the config, --download is gone from the README, and gym dataset collate
runs from a clean clone with no network. (If maintainers prefer HF hosting for the two
splits, we're happy to move them post-review — the in-tree layout makes the PR
verifiable today.)

B2 (SEQ_STRICT). The contradiction was ours: the README said "default False" while
the code ships True. Resolved by documenting reality: SEQ_STRICT = True is the
shipped setting and the one used for every reported number (the 300/300 gold replay,
the example rollouts, the reference GRPO run). The README now states this explicitly
and notes that tau2-comparable set-based scoring requires setting it to False; the
"parity" wording is scoped to the component decomposition, not pass rates.

B3 (db/kb story). One story everywhere now: everything ships in-tree; nothing is
downloaded before serving. Config comments, README (both places), and the PR body were
aligned.

Correctness bugs

C1. matches() now consults the gold replay's per-action error flags (computed once
and shared with the DB check): an errored predicted call satisfies a gold action only if
that gold action also errors on its own replay — three in-tree tasks deliberately
exercise error paths and still pass. Errored writes no longer count as wrong writes
(they mutated nothing, and penalising retry-after-error seemed worse). Your
INSUFFICIENT_FUNDS repro now scores strict=0. Test:
test_errored_call_does_not_satisfy_action_check.

C2. reset() clears world["calls"] (and transferred) after running
initialization_actions, so init writes can never count against ACTION or purity; a
failing init action now raises instead of silently seeding a broken world. Test:
test_init_actions_do_not_pollute_calls.

C3. In lenient mode the text of a mixed turn is appended to the customer-facing
history before the tools run — the simulator, COMMUNICATE check and judge all see it;
nothing is silently dropped. Test: test_lenient_mixed_turn_keeps_text_in_history.

[shashank3959]

Please add a manifest.yaml entry under environments/

Reference: https://docs.nvidia.com/nemo/gym/main/contribute/environments/new-environment/#environment-manifest

[shashank3959]

To understand correctly, NL_ASSERTION can never fail strict success.

For the 13 {DB, NL_ASSERTION} tasks — all abstention scenarios with no gold actions — DB correctly passes a do-nothing agent, so the judge is the only check that can tell a correct refusal-with-explanation from a mute agent. It has no power to. A do-nothing, say-nothing trajectory scores 0.925 with a zero judge and 1.0 when the judge is unavailable, since judge_short is 0 for None.

Ex: esc_legit_transfer_locked_1 is the clearest case: the task requires a hand-off, and never transferring still scores 0.925.

[shashank3959]

Essentially, for these kind of tasks, make sure they have some deterministic requirement that can still fail and provide an informative reward signal.

[krishnpci]

Fixed on the new head. strict now requires at least one non-empty customer-facing message, so the trajectory you demonstrated no longer passes. Tasks can also declare deterministic requirements in evaluation_criteria: max_tool_calls and require_transfer. esc_legit_transfer_locked_1 now carries require_transfer plus a gold transfer_to_human_agents action, so never transferring fails strict. Regression tests cover all three mechanisms.

[shashank3959]

_transcript() strips all tool traffic, so no assertion about tool behaviour is observable to the judge. Most such assertions have a deterministic backstop, but two have none:

1. chit_greeting_smalltalk_1 ("Agent makes no tool calls"): read calls do not touch the DB and extra lookups are not charged, so 0, 1, or 3 tool calls all score 1.0 on identical text.

2. esc_legit_transfer_locked_1 ("transfers instead of improvising"): world["transferred"] is assigned in three places and never read by any scoring code. Transferring and not transferring both score 1.0.

[krishnpci]

Both cases have a deterministic check now. chit_greeting_smalltalk_1 sets max_tool_calls: 0, so any tool call fails strict regardless of the text. esc_legit_transfer_locked_1 requires the transfer, and world["transferred"] is now read by scoring. You were right that it was write-only before.

[shashank3959]

is it ever possible that the tool modifies the db, but leads to a tool exception (due to whatever reason) in the subsequent steps.

It doesn't seem to roll the db back to the original pre-call state?

[shashank3959]

For example, set_card_controls can apply a valid ATM update, encounter malformed online data, and return error=True while retaining the ATM mutation.

[krishnpci]

Correct, this could happen. Fixed at the engine level rather than per tool: apply_tool snapshots the episode db before any write or soft-write tool and restores it when the call reports an error, so the set_card_controls case leaves no trace. Reads skip the snapshot. A regression test covers your exact scenario

[shashank3959]

Would this make an unexpected failure in the gold trajectory become accepted behavior?

[shashank3959]

These two tasks cannot achieve the outcome they require:

• ch3_rd_close_open_fd_1 has ₹92,854.11 available but attempts to book a ₹1,00,000 FD.
• ch5_deposit_restructure_1 has ₹2,02,476.85 available but attempts to book a ₹3,00,000 FD.
  Both create_fd calls therefore return INSUFFICIENT_FUNDS. However, because the same failure occurs during gold replay, the scorer treats it as correct and awards strict=1.0 and score=1.0, even though the tasks require successful FD booking by its language.

Please either adjust the balances or FD amounts so booking can succeed, or rewrite these as intentional insufficient-funds tasks. Expected errors should also be marked explicitly rather than inferred from a failing gold replay.

[shashank3959]

These two tasks cannot achieve the outcome they require:

• ch3_rd_close_open_fd_1 has ₹92,854.11 available but attempts to book a ₹1,00,000 FD.
• ch5_deposit_restructure_1 has ₹2,02,476.85 available but attempts to book a ₹3,00,000 FD.
  Both create_fd calls therefore return INSUFFICIENT_FUNDS. However, because the same failure occurs during gold replay, the scorer treats it as correct and awards strict=1.0 and score=1.0, even though the tasks require successful FD booking.

[krishnpci]

Both are fixed. ch3_rd_close_open_fd_1 now books a 75,000 FD against 92,854.11 available after the RD closure payout, and ch5_deposit_restructure_1 books a 36-month 2,00,000 FD against 2,02,476.85 available after the FD948082 closure credits 52,476.85. The amounts are updated consistently in the gold actions, known_info and task_instructions, both bookings are expected to succeed rather than error, and expected errors elsewhere are now marked explicitly with expect_error: true instead of being inferred from the replay. Both tasks replay at exactly 1.0, and ch5 also passed a live end-to-end run.

[shashank3959]

good to add allowed_agents: [gymnasium_agent]

This resources server only supports the Gymnasium /reset and /step protocol; its inherited /verify method raises NotImplementedError. Without allowed_agents, Gym treats it as compatible with any agent harness, so it can be paired with simple_agent or another unsupported agent and fail only at runtime. Please add allowed_agents: [gymnasium_agent] so incompatible pairings fail during configuration.

Example

indian_banking:
  resources_servers:
    indian_banking:
      allowed_agents: [gymnasium_agent]
      entrypoint: app.py
      domain: agent

[krishnpci]

Added in environments/indian_banking/ with the config and task_data.py schema.

[shashank3959]

Another potential scoring issue:

action comparator uses three incompatible multiplicity semantics (core/reward.py)

_score_actions computes three different match signals over the same gold/call lists, and they disagree on whether a repeated gold action counts once or twice:

• action_frac / bad_writes (lines 135-143, 160-171) use any(matches(gold, c) for c in calls) — set-based, multiplicity-blind.
• seq_frac (lines 147-158) is a proper LCS — multiplicity-strict.
• _gold_order_violated (lines 187-201) does break on the first matching call for every gold entry, never marking a call as consumed.

67 of 300 tasks legitimately repeat a gold action (a read-after-write verification step). For these tasks:

• _gold_order_violated maps both the original read and the verification read to the same earliest matching call, since it never consumes matches. This falsely flags the trajectory as out-of-order and applies the full EFF_W_ORDER (0.2) penalty — in all 67/67 cases.
• _efficiency_cost flags the verification read as a duplicate call (its seen set is keyed on name+args only).
• Combined, a byte-perfect trajectory tops out at 0.955, never 1.0.
• Because SEQ_STRICT = True (line 53) requires seq_frac >= 1.0, an agent that correctly skips the redundant repeat (matching real banking-agent behavior) fails strict outright — despite action_frac == 1.0 and a fully correct DB state.

[krishnpci]

Another potential scoring issue:

action comparator uses three incompatible multiplicity semantics (core/reward.py)

_score_actions computes three different match signals over the same gold/call lists, and they disagree on whether a repeated gold action counts once or twice:

• action_frac / bad_writes (lines 135-143, 160-171) use any(matches(gold, c) for c in calls) — set-based, multiplicity-blind.
• seq_frac (lines 147-158) is a proper LCS — multiplicity-strict.
• _gold_order_violated (lines 187-201) does break on the first matching call for every gold entry, never marking a call as consumed.

67 of 300 tasks legitimately repeat a gold action (a read-after-write verification step). For these tasks:

• _gold_order_violated maps both the original read and the verification read to the same earliest matching call, since it never consumes matches. This falsely flags the trajectory as out-of-order and applies the full EFF_W_ORDER (0.2) penalty — in all 67/67 cases.
• _efficiency_cost flags the verification read as a duplicate call (its seen set is keyed on name+args only).
• Combined, a byte-perfect trajectory tops out at 0.955, never 1.0.
• Because SEQ_STRICT = True (line 53) requires seq_frac >= 1.0, an agent that correctly skips the redundant repeat (matching real banking-agent behavior) fails strict outright — despite action_frac == 1.0 and a fully correct DB state.

@shashank3959 thank you for the careful review, these were valuable catches and each one is addressed on the new head. The branch is also rebased onto current main.

On the reward side: strict now requires at least one non-empty customer-facing message, which closes the 0.925 case you demonstrated on the abstention tasks. Tasks can additionally declare max_tool_calls (chit_greeting_smalltalk_1 uses 0) and require_transfer (esc_legit_transfer_locked_1, which also carries a gold transfer_to_human_agents action), and world["transferred"] is now read by scoring as you suggested. The action comparator uses a single matcher for action_frac, ordering and duplicate detection: each gold action is assigned its own call through a maximum bipartite matching, so a trajectory that reproduces gold exactly scores exactly 1.0, the false out-of-order and duplicate charges on the 67 repeat-gold tasks are gone, and skipping a required repeat lowers action_frac and seq_frac consistently.

On the engine and data side: tool writes are transactional now, with the episode db snapshotted before write tools and restored when a call reports an error, so the set_card_controls partial-update case leaves no trace. Both FD tasks are feasible (ch3 books 75,000 and ch5 books 2,00,000, comfortably within the balances once the closure proceeds land), and expected errors are declared rather than inferred: an errored call matches a gold action only when it carries expect_error: true, used only by wi_nonexistent_sr_1. A new shipped-data test replays all 300 tasks in CI and fails if a gold call errors without the marking, a marked call does not error, or any replay scores below exactly 1.0. The example rollouts were re-collected under the current scorer so the shipped artifacts match the shipped reward code.

Structurally, allowed_agents: [gymnasium_agent] is added to the config as suggested, and there is now an environments/indian_banking/ entry with manifest.yaml and config.yaml (gym env validate passes with the declared profile matching the inferred custom-gym-agent-loop) plus a task_data.py schema, so the repo drift test validates every committed row.

Validation on this head: 31 server tests including the 300-task replay check, the task_data and manifest suites, gym env test, and pre-commit --all-files are all green, and every commit is DCO-signed. Happy to adjust anything further.

[shashank3959]

@krishnpci - Is it possible to move the data (~8.8MB blob) to huggingface and download it, instead of storing it in tree?

I think, the B1 blocker comment reported here #2881 (comment) was meant more to ensure the data is possible to download / made public

[krishnpci]

@shashank3959 , these datasets are entirely synthetic, I have made few changes it should work, without affecting the functionality, will put in huggingface if we still face issues. Can this be triggered again.

[shashank3959]

This is a shared repo-wide file, and we'd prefer it were not updated to pass the CI actually.

From what I understand, the initial commits already had the data in Hugging Face, and if made public it would resolve this?

[krishnpci]

yeah, I understood, let me see how to open the huggingface one as it might need some approvals, I have reverted to the approved head.

[krishnpci]

@krishnpci - Is it possible to move the data (~8.8MB blob) to huggingface and download it, instead of storing it in tree?

I think, the B1 blocker comment reported here #2881 (comment) was meant more to ensure the data is possible to download / made public

@shashank3959 Done as you suggested. The dataset artifacts are out of the tree and the shared secrets baseline is untouched: train.jsonl, validation.jsonl, db.json and kb.json now live on the Hugging Face dataset repo NPCI/nemo-gym-indian-banking (Apache 2.0), byte-identical to the reviewed revision, and only example.jsonl plus the metrics stay committed. The configs carry the source blocks again, so gym dataset collate --download fetches the splits, and the README documents the one-time hf download for db.json and kb.json. I verified the full round trip locally: a fresh checkout downloads everything byte-identically, all 31 server tests and gym env test pass, and the secrets detector passes on the resulting diff with no baseline change.

The repo is uploaded and is public. The earlier approvals from you was auto-dismissed by the interim pushes, so whenever it suits you this is ready for a re-look, and the CI can be triggered on the new head.

[krishnpci]

@krishnpci - Is it possible to move the data (~8.8MB blob) to huggingface and download it, instead of storing it in tree?
I think, the B1 blocker comment reported here #2881 (comment) was meant more to ensure the data is possible to download / made public

@shashank3959 Done as you suggested. The dataset artifacts are out of the tree and the shared secrets baseline is untouched: train.jsonl, validation.jsonl, db.json and kb.json now live on the Hugging Face dataset repo NPCI/nemo-gym-indian-banking (Apache 2.0), byte-identical to the reviewed revision, and only example.jsonl plus the metrics stay committed. The configs carry the source blocks again, so gym dataset collate --download fetches the splits, and the README documents the one-time hf download for db.json and kb.json. I verified the full round trip locally: a fresh checkout downloads everything byte-identically, all 31 server tests and gym env test pass, and the secrets detector passes on the resulting diff with no baseline change.

The repo is uploaded and is public. The earlier approvals from you was auto-dismissed by the interim pushes, so whenever it suits you this is ready for a re-look, and the CI can be triggered on the new head.

@shashank3959 all the changes have been done, can you check this once for approval, the CI can be triggered as well on this.

[shashank3959]

/ok to test 171f827

[krishnpci]

@shashank3959 thanks a lot !