feat(m3): add capability_4 data and enable testing real follow-up questions

[haroldship]

Feature Pull Request

Related Issue

Closes #125

Description

M3 only had one small dataset (small_train.zip) and no capability_4 data at all, so there was no way to run the M3 (Vakra) benchmark against capability_4. On top of that, the eval loop couldn't test real follow-up questions: for a multi-turn sample it re-invoked the agent on every turn in sequence, starting from nothing each time. That means a "follow-up" question was never actually a follow-up — the agent had to re-solve earlier turns whose answers were already known, instead of treating them as established context and answering only the new question.

This PR adds the missing dataset and fixes that gap:

• Adds capability_4_multiturn_policy_sampled.zip (300 samples, VAKRA-licensed the same way as small_train.zip), and removes four old JSON files from benchmarks/m3/data that had no documented source or license.
• Adds a script that generates --eval-key shortcuts (single-turn, multi-turn, with-policy, without-policy) for any M3 zip, and uses it for this dataset.
• Adds the m3_task_4 entry to the registry config — without it, --capability m3_task_4 couldn't find any containers even though the container itself worked fine.
• Wires VAKRA's per-sample instructions (additional_instructions) into the agent for that task only.
• Adds dialogue priming: for a multi-turn sample, every turn except the last already has a known answer. Those earlier turns are now fed into the agent as already-answered conversation history (a normal user/assistant back-and-forth), and only the final, unanswered turn is actually sent to the agent and scored. That final turn is now a real follow-up question, asked with the earlier context already established, instead of a fresh multi-step task.
• Along the way, this surfaced a data bug: this dataset's output files label things output/sequence, while the loader only recognized small_train.zip's ground_truth/gold_sequence names, so the correct-answer data for scoring was silently empty. Fixed to accept both.
• This branch also includes the already-open registry-precedence fix ([Bug]: Every benchmark silently gets zero tools because a leftover global.env setting fights each benchmark's own config #124 / PR fix(config): benchmarks silently got zero tools because of a leftover setting in global.env #126), merged in because testing this feature required it.
• Also fixes a CI test (test_m3_data_loader_loads_bundled_default_matching_registry) that broke as a side effect of adding m3_task_4 to the registry — it was checking every capability in the registry against small_train.zip, which never covered capability 4 in the first place. Scoped it to only check the capabilities small_train.zip actually has.

Test & Results

Ran the same task 5 times in a row with compare.sh, to see whether the follow-up question could actually be answered correctly and how consistently:

./benchmarks/m3/compare.sh --runs 5 \
  --m3-data benchmarks/m3/data/capability_4_multiturn_policy_sampled.zip \
  --eval-key cap4_multi_turn_with_policy_s1 \
  --capability m3_task_4 \
  --domain books \
  --no-policies

Result: 2 out of 5 runs passed (Pass@1 = 40%, pass@5 = 100% — at least one clean pass).

Run	R1	R2	R3	R4	R5
Result	✓	✓	✗	✗	✗

Checked the raw trace on a passing run to confirm this wasn't just "the plumbing runs without crashing" — the earlier turn's answer really does show up as prior assistant history, the live turn's message carries the per-task policy text, and the agent calls the real domain tool (books_get_order_dates_by_customer) with the correct arguments and gets back the result that matches the gold answer exactly. The 3 failing runs are ordinary model misses on this task, not the dataset/priming code doing anything wrong — before the registry fix (#124) was merged into this branch, every run failed the same way regardless of the question, because the agent had no tools at all.

Type of Changes

• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Testing

• I have tested this feature locally
• I have added tests that prove my feature works
• All new and existing tests passed (just lint, pytest benchmarks/m3/tests/ -m sanity)

Documentation

• I have updated the documentation accordingly
• I have added docstrings to new functions/classes

Checklist

• My code follows the code style of this project
• I have performed a self-review of my own code
• I have made corresponding changes to the documentation

[coderabbitai]

📝 Walkthrough

Walkthrough

Capability 4 M3 data and evaluation-key sets are added, multiturn evaluation now primes prior turns before evaluating the final turn, alternate gold formats are supported, and runtime, licensing, and environment configuration are updated.

Changes

Capability 4 M3 integration

Layer / File(s)	Summary
Dataset packaging and evaluation partitions .gitignore, .pre-commit-config.yaml, LICENSE, benchmarks/m3/data/NOTICE, benchmarks/m3/data/address.json, benchmarks/m3/data/olympics.json, benchmarks/m3/eval_config.toml	The capability-4 dataset is unignored, excluded from large-file checks, documented under its separate license, and represented by turn/policy-based evaluation-key lists; obsolete address and Olympics fixtures are removed.
M3 data loading and key generation benchmarks/m3/m3_data_loader.py, benchmarks/m3/scripts/generate_policy_turn_keys.py	Gold extraction accepts ground_truth or output formats, while a new CLI generates deterministic single-/multiturn and policy-based sample buckets.
Multiturn priming and final-turn evaluation benchmarks/helpers/sdk_eval_helpers.py, benchmarks/m3/eval_m3.py	Prior dialogue turns are passed as message history, policy context is propagated, only the final turn is evaluated for multiturn samples, and expected outputs and diffs use that final turn.
Capability 4 runtime configuration benchmarks/m3/config/m3_registry_m3_data.yaml, config/global.env	The capability-4 multiturn service is registered with its supported domains, and the global registry default is replaced with benchmark-specific configuration guidance.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related issues

• [Feature]: Add capability_4 M3 dataset and clean up unlicensed legacy data files #125 — Covers the same capability-4 dataset integration, registry/eval-key additions, legacy data cleanup, and policy-aware evaluation changes.
• [Feature]: add capability_4 M3 dataset and rationalize benchmarks/m3/data #123 — Covers the capability-4 dataset, eval-key generation, licensing updates, and obsolete M3 data cleanup.

Possibly related PRs

• cuga-project/cuga-eval#105 — Modifies the same evaluation helper and evaluator paths for incremental or resumable evaluation behavior.
• cuga-project/cuga-eval#126 — Makes the same global environment change removing the registry default and documenting per-benchmark configuration.

Sequence Diagram(s)

sequenceDiagram
  participant M3Evaluator
  participant EvaluationHelper
  participant Agent
  M3Evaluator->>M3Evaluator: Build prior-turn message history
  M3Evaluator->>EvaluationHelper: Submit history and final intent
  EvaluationHelper->>Agent: Invoke with primed messages
  Agent-->>EvaluationHelper: Return final-turn response
  EvaluationHelper-->>M3Evaluator: Return evaluation result

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: adding capability_4 data and enabling real multi-turn follow-up evaluation.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/125-capability4-m3-dataset

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

benchmarks/helpers/sdk_eval_helpers.py (1)

843-876: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

GenericReactAgent drops primed history history_messages are ignored on the React path because GenericReactAgent.invoke() only uses messages[-1]. Follow-up tasks won’t see the intended conversation context; either thread the full message list into _build_initial_messages() or disable priming for --agent react.

🤖 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 `@benchmarks/helpers/sdk_eval_helpers.py` around lines 843 - 876, Update the
React execution path around GenericReactAgent.invoke() so history_messages are
preserved as conversation context instead of passing or processing only
messages_to_send[-1]. Thread the complete messages_to_send list through
_build_initial_messages() or disable history priming when the selected agent is
react, ensuring follow-up tasks do not claim unsupported primed context.

🧹 Nitpick comments (1)

benchmarks/m3/m3_data_loader.py (1)

220-221: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Defensively check that gt is a dictionary.

While the current capability 4 datasets are well-formed, it's safer to ensure that items in gt_list are dictionaries before invoking .get(). This prevents an AttributeError if a future or malformed dataset contains null or string entries in the array, aligning with the defensive type-checking used elsewhere in this function.

♻️ Proposed refactor

             if isinstance(gt_list, list):
-                gt_by_turn = {gt.get("turn_id", i): gt for i, gt in enumerate(gt_list)}
+                gt_by_turn = {gt.get("turn_id", i): gt for i, gt in enumerate(gt_list) if isinstance(gt, dict)}
                 for i, _turn in enumerate(turns):

🤖 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 `@benchmarks/m3/m3_data_loader.py` around lines 220 - 221, Update the
gt_by_turn construction in the gt_list handling to call get only for dictionary
entries, while safely ignoring or otherwise handling null and non-dictionary
items without raising AttributeError. Preserve the existing turn_id fallback
behavior for valid dictionary entries.

🤖 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.

Outside diff comments:
In `@benchmarks/helpers/sdk_eval_helpers.py`:
- Around line 843-876: Update the React execution path around
GenericReactAgent.invoke() so history_messages are preserved as conversation
context instead of passing or processing only messages_to_send[-1]. Thread the
complete messages_to_send list through _build_initial_messages() or disable
history priming when the selected agent is react, ensuring follow-up tasks do
not claim unsupported primed context.

---

Nitpick comments:
In `@benchmarks/m3/m3_data_loader.py`:
- Around line 220-221: Update the gt_by_turn construction in the gt_list
handling to call get only for dictionary entries, while safely ignoring or
otherwise handling null and non-dictionary items without raising AttributeError.
Preserve the existing turn_id fallback behavior for valid dictionary entries.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 94680029-61ed-41fa-94a6-e360514dc910

📥 Commits

Reviewing files that changed from the base of the PR and between 5562399 and 49e8506.

⛔ Files ignored due to path filters (1)

• benchmarks/m3/data/capability_4_multiturn_policy_sampled.zip is excluded by !**/*.zip

📒 Files selected for processing (15)

• .gitignore
• .pre-commit-config.yaml
• LICENSE
• benchmarks/helpers/sdk_eval_helpers.py
• benchmarks/m3/config/m3_registry_m3_data.yaml
• benchmarks/m3/data/NOTICE
• benchmarks/m3/data/address.json
• benchmarks/m3/data/hockey.json
• benchmarks/m3/data/olympics.json
• benchmarks/m3/data/olympics_multiturn.json
• benchmarks/m3/eval_config.toml
• benchmarks/m3/eval_m3.py
• benchmarks/m3/m3_data_loader.py
• benchmarks/m3/scripts/generate_policy_turn_keys.py
• config/global.env

💤 Files with no reviewable changes (4)

• benchmarks/m3/data/olympics_multiturn.json
• benchmarks/m3/data/olympics.json
• benchmarks/m3/data/address.json
• benchmarks/m3/data/hockey.json

[haroldship]

Replying to the CodeRabbit review (both items were addressed in 66735fe; comments were posted body-only because they fell outside the diff, so replying here):

GenericReactAgent drops primed history — history_messages are ignored on the React path because GenericReactAgent.invoke() only uses messages[-1].

Fixed in 66735fe by taking the "disable priming for react" option: evaluate_task now raises NotImplementedError if history_messages is passed together with a GenericReactAgent, so primed turns can never be silently dropped. The combination is not currently reachable — M3's --agent react goes through the separate eval_m3_react.py, which never calls the multiturn path — but since this is a shared helper, failing loudly protects other callers. Threading the full message list into _build_initial_messages() was deliberately not done: that changes the React agent's behavior and is out of scope for this dataset PR.

Defensively check that gt is a dictionary. It's safer to ensure that items in gt_list are dictionaries before invoking .get().

Fixed in 66735fe exactly as suggested: the gt_by_turn comprehension now filters with if isinstance(gt, dict), so null/string entries in a malformed gold list are skipped (that turn falls through to the existing empty-gold path) instead of raising AttributeError. The turn_id-with-index-fallback behavior for valid dict entries is preserved.

Verified at the current tip: uv run pytest benchmarks/m3/tests -q -x → 42 passed, 2 expected skips.