From afc9b96bb979eaf9cca48a789ddb439890921648 Mon Sep 17 00:00:00 2001 From: Panpan Xu Date: Wed, 6 May 2026 00:11:02 +0000 Subject: [PATCH 1/5] docs(roadmap): add committed core-api-rename plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Starts tracking docs/roadmap/committed/ for approved design plans. Drafts (docs/roadmap/draft/), PR-body scratchpads (docs/pr/), and personal-tooling output (docs/superpowers/) stay untracked by .gitignore so ad-hoc thinking doesn't accidentally land in the repo. The core-api-rename plan covers the 0.2.0 breaking rename: app.py/client.py/reward_function.py -> rollout_server.py/ rollout_launcher.py/reward.py, RolloutClient -> RolloutLauncher (plus BatchResult/BatchItem renames), and the payload["_rollout"] -> payload["rollout_config"] flip. No shims — single PR when implemented. Co-Authored-By: Claude Opus 4.7 --- .gitignore | 5 + docs/roadmap/committed/core-api-rename.md | 232 ++++++++++++++++++++++ 2 files changed, 237 insertions(+) create mode 100644 docs/roadmap/committed/core-api-rename.md diff --git a/.gitignore b/.gitignore index 1890f79..85326ee 100644 --- a/.gitignore +++ b/.gitignore @@ -25,3 +25,8 @@ config.toml .claude/ .bedrock_agentcore.yaml + +# Roadmap drafts and ad-hoc doc scratchpads — only docs/roadmap/committed/ is tracked +docs/roadmap/draft/ +docs/pr/ +docs/superpowers/ diff --git a/docs/roadmap/committed/core-api-rename.md b/docs/roadmap/committed/core-api-rename.md new file mode 100644 index 0000000..788db36 --- /dev/null +++ b/docs/roadmap/committed/core-api-rename.md @@ -0,0 +1,232 @@ +--- +title: "Core API rename: rollout-server / rollout-launcher / rollout-config" +description: "ART's public surface uses generic names (app.py, client.py," +--- +# Core API rename: rollout-server / rollout-launcher / rollout-config + +## Summary + +ART's public surface uses generic names (`app.py`, `client.py`, +`RolloutClient`, `payload["_rollout"]`) that don't convey the +two-sided architecture. This roadmap renames module files, collector-side +classes, and the rollout payload schema in a single breaking change, so +the `ls` view, the `import` view, and the rollout-body view all use +the same vocabulary. + +**Breaking change, one PR, one version bump (0.2.0).** No shims, no +deprecation warnings — users update imports / payload keys or pin +`<0.2`. At this stage of the toolkit's adoption, the cost of carrying +deprecation scaffolding outweighs the migration burden. + +`AgentCoreRLApp`, `RolloutFuture`, and `RewardFunction` stay +unchanged. + +### Changes at a glance + +**Module files:** + +| Today | Renamed | +|---|---| +| `app.py` | `rollout_server.py` | +| `client.py` | `rollout_launcher.py` | +| `reward_function.py` | `reward.py` | + +**Public classes:** + +| Today | Renamed | +|---|---| +| `RolloutClient` | `RolloutLauncher` | +| `BatchResult` | `RolloutBatch` | +| `AsyncBatchResult` | `AsyncRolloutBatch` | +| `BatchItem` | `RolloutResult` | + +**Rollout payload:** + +| Today | Renamed | +|---|---| +| `payload["_rollout"]` | `payload["rollout_config"]` | +| _(user hardcodes `api_key="EMPTY"`)_ | `rollout_config["api_key"]` (backend-injected) | + +--- + +## Why + +**Generic filenames.** `app.py` / `client.py` could live in any +Python project; nothing in the directory listing indicates ART's +two-sided architecture. + +**Generic class names.** `RolloutClient` reads as an HTTP client; +`BatchItem`/`BatchResult` read as generic dataclasses and bury the +`run_batch` relationship. The `Item → .result → Result` hop is +confusing. + +**Misleading payload conventions.** `_rollout` is the one required +public field in every rollout entrypoint, but the leading underscore +signals "private, don't touch." And the name doesn't describe the +contents — it's a settings bundle (`base_url`, `model_id`, +`sampling_params`, `session_id`, `input_id`, `exp_id`, `s3_bucket`), +not a rollout. + +**`api_key="EMPTY"` hardcoded literal.** The OpenAI SDK requires a +non-empty `api_key`; the vLLM/SGLang ecosystem convention is to pass +`"EMPTY"` when the target doesn't check auth. Not ART's invention, +but surfacing in every user's code is avoidable. Backend injection +(`"EMPTY"` for local; real key for Tinker) keeps the literal out of +user files. + +**One rename, not three.** Splitting across PRs leaves users looking +at renamed classes inside `app.py`, or old payload keys in renamed +files. Coherence requires a single pass. + +## Post-rename mental model + +> ART has two sides of one wire. +> +> The **rollout side** runs on ACR: an `AgentCoreRLApp` with a +> `@rollout_entrypoint` handler (in `rollout_server.py`). It reads +> per-rollout settings from `payload["rollout_config"]` (`base_url`, +> `model_id`, `api_key`, `sampling_params`), runs the agent, +> computes a reward, returns a dict. Results save to S3. +> +> The **launcher side** runs in your trainer or evaluator: a +> `RolloutLauncher` submits rollouts and collects results (in +> `rollout_launcher.py`). `launcher.invoke(...)` returns a +> `RolloutFuture`; `launcher.run_batch(...)` returns a `RolloutBatch` +> whose entries are `RolloutResult`s. +> +> You subclass `RewardFunction` (in `reward.py`) for scoring, called +> inside the rollout. + +`ls src/agentcore_rl_toolkit/` after the rename: + +``` +__init__.py +rollout_server.py # AgentCoreRLApp +rollout_launcher.py # RolloutLauncher, RolloutFuture, RolloutBatch, ... +reward.py # RewardFunction +backends/ +frameworks/ +``` + +Class-level asymmetry (`App` vs `Launcher`) is kept deliberately: +the App subclasses an AWS-managed runtime; the Launcher is a plain +Python object. File-level symmetry (`rollout_server.py` vs +`rollout_launcher.py`) shows the pairing on `ls`. + +## Migration + +The exact renames are already listed in the [Changes at a glance](#changes-at-a-glance) tables. Two points worth stating explicitly: + +- **Missing `api_key` in `rollout_config` raises**, not defaults. Trainers set it themselves — slime ships `"EMPTY"` (gateway in front of unauth'd SGLang); rllm ships a real key when routing through Tinker. +- `AgentCoreRLApp`-level import paths stay at the package root: `from agentcore_rl_toolkit import AgentCoreRLApp` keeps working; only `from agentcore_rl_toolkit.app import …` breaks. + +Users who can't migrate pin `agentcore-rl-toolkit<0.2`. At this adoption stage we don't ship a dedicated migration guide — the Changes-at-a-glance tables plus release notes are enough. + +## Backend trainer integration impact + +The rename touches three backend integrations differently depending on +where their code lives. Summary table first, details below. + +| Backend | Code location | Touched by this PR? | Required downstream work | +|---|---|---|---| +| **slime** | in-tree (`src/agentcore_rl_toolkit/backends/slime/`) | ✅ yes | none — bundled in the 0.2.0 PR | +| **rllm** | out-of-tree ([rllm-org/rllm](https://github.com/rllm-org/rllm), `rllm/experimental/engine/remote_runtime/agentcore_runtime.py`) | ❌ no | ~5-line patch in rllm repo; land there when they bump their ART pin to `>=0.2` | +| **verl** | not integrated yet | ❌ no | pending investigation — patch to be integrated into the art codebase as part of the verl integration work | + +### slime (in-tree) + +Two touch sites, both handled by the rename PR: `RolloutClient` → `RolloutLauncher` in `backends/slime/integration/rollout.py` (import + constructor). Slime never touches `payload["_rollout"]` directly — it passes kwargs through `invoke_async(...)`, so the payload-key flip is contained in the renamed `rollout_launcher.py` that slime consumes. Slime-backend users bump their `agentcore-rl-toolkit` pin and keep going. + +### rllm (out-of-tree) + +Lives in [rllm-org/rllm](https://github.com/rllm-org/rllm) at `rllm/experimental/engine/remote_runtime/agentcore_runtime.py`. File a tracking issue on that repo when 0.2.0 ships; changes needed there: + +1. `RolloutClient` → `RolloutLauncher` (import + constructor). +2. Audit for any direct `payload["_rollout"]` writes (shouldn't exist — the launcher builds it). +3. For Tinker/real-API-key paths, pass via `invoke(..., api_key=)` so it lands in `rollout_config["api_key"]`. + +Until they update, rllm pinned to `agentcore-rl-toolkit<0.2` continues to work. + +### verl (pending investigation) + +Not integrated in-tree or out-of-tree today. When integration work starts, the adapter gets written against 0.2+ directly and lands under `src/agentcore_rl_toolkit/backends/verl/`. Open a tracking issue for scope visibility; this PR neither blocks nor pre-empts it. + +## Out of scope + +- Renaming `AgentCoreRLApp`, `RolloutFuture`, `RewardFunction`. +- Changing method names on the launcher. +- Renames for backend module paths (e.g. `agentcore_rl_toolkit.backends.slime.integration.rollout.generate_rollout`) — those are used as strings in user `train.sh` scripts and in out-of-tree backends' code. +- Eliminating `"EMPTY"` itself. The SDK still requires non-empty + `api_key`; we only stop surfacing the literal in user code. +- Promoting `rollout_config` to a `TypedDict` or dataclass. + +## Open questions + +1. `RolloutResult.result` — keep `.result` or rename to `.data` to + avoid `rollout_result.result`? Recommend: keep `.result`; revisit + if users complain. +2. `api_key` naming — bare or scoped (`inference_api_key`)? Recommend: + bare; nesting under `rollout_config` makes scope clear and matches + the OpenAI-SDK argument name. +3. Structured payload — promote to `TypedDict`? Recommend: stay dict + for 0.2; consider follow-up if users ask for autocomplete. + +--- + +## Task checklist (single PR, 0.2.0) + +**Core renames** +- [ ] `git mv src/agentcore_rl_toolkit/app.py → rollout_server.py` + (keeps `AgentCoreRLApp`). +- [ ] `git mv src/agentcore_rl_toolkit/client.py → rollout_launcher.py` + and rename `RolloutClient` → `RolloutLauncher`, + `BatchResult` → `RolloutBatch`, + `AsyncBatchResult` → `AsyncRolloutBatch`, + `BatchItem` → `RolloutResult`. +- [ ] `git mv src/agentcore_rl_toolkit/reward_function.py → reward.py` + (keeps `RewardFunction`). +- [ ] Update `agentcore_rl_toolkit/__init__.py` — only new names in + `__all__`. + +**Payload** +- [ ] `AgentCoreRLApp.rollout_entrypoint`: read `rollout_config` only; + raise `KeyError` if missing `api_key`. +- [ ] `RolloutLauncher`: inject `rollout_config` (with `api_key`) on + send; drop the `_rollout` code path. + +**Consumers (same PR)** +- [ ] Update examples: `strands_math_agent`, `strands_appworld_agent`, + `strands_migration_agent`, `strands_officebench_agent`. + - [ ] Swap imports and class names. + - [ ] Swap `payload["_rollout"]` → `payload["rollout_config"]`. + - [ ] Replace `api_key="EMPTY"` literal with `cfg["api_key"]`. +- [ ] Update the in-tree slime backend + (`backends/slime/integration/rollout.py`): 2-line import/constructor + swap. +- [ ] File a tracking issue on rllm-org/rllm (see [rllm section](#rllm-out-of-tree)). + +**Docs** +- [ ] Update `docs/site/scripts/gen_api.py` `MODULES` allowlist. +- [ ] Regenerate API reference (`pnpm gen:api`); commit diff. +- [ ] Update the Overview guide with the new mental-model paragraph. +- [ ] Update the prepare-agent-for-RL guide's canonical code block. + +**Release** +- [ ] Bump version to `0.2.0` in `pyproject.toml`. +- [ ] Release notes for 0.2.0 (breaking change) — call out the renames + + `rollout_config` payload key. + +**Tests** +- [ ] New imports resolve (`from agentcore_rl_toolkit import + RolloutLauncher`). +- [ ] Old imports fail loudly (`ImportError`, not silent alias). +- [ ] `payload["rollout_config"]` round-trips launcher → server. +- [ ] Missing `api_key` raises a clear `KeyError`, not defaults to + `"EMPTY"` silently. + +## Milestones + +1. **M1** — spike in a throwaway branch; size the diff. +2. **M2** — PR lands on `main`; 0.2.0 released. + +M1 ≈ half day. M2 ≈ 1 day. From 6338fba3f4978c165af04ab19e0fdb5c74d3b33f Mon Sep 17 00:00:00 2001 From: Panpan Xu Date: Wed, 6 May 2026 00:11:02 +0000 Subject: [PATCH 2/5] docs(roadmap): add slime data-contract and SlimeRunner plans Commits two approved plans to docs/roadmap/committed/: - slime-data-contract.md: make the JSONL row's metadata dict the agent payload verbatim, so non-GSM8K agents train without slime-integration changes. Includes Appendix A on slime's group_index assignment. - slime-runner-entrypoint.md: wrap train.sh + config.yaml behind a single SlimeRunner Python class as the primary entry point; train.sh stays as the escape hatch. Co-Authored-By: Claude Opus 4.7 --- docs/roadmap/committed/slime-data-contract.md | 601 ++++++++++++++++++ .../committed/slime-runner-entrypoint.md | 415 ++++++++++++ 2 files changed, 1016 insertions(+) create mode 100644 docs/roadmap/committed/slime-data-contract.md create mode 100644 docs/roadmap/committed/slime-runner-entrypoint.md diff --git a/docs/roadmap/committed/slime-data-contract.md b/docs/roadmap/committed/slime-data-contract.md new file mode 100644 index 0000000..cbe3e17 --- /dev/null +++ b/docs/roadmap/committed/slime-data-contract.md @@ -0,0 +1,601 @@ +--- +title: "Slime backend: use-case-independent rollout payload" +description: "Make the JSONL row's `metadata` field the agent payload verbatim, so migration/appworld/officebench agents can train via slime without any slime integration changes." +--- +# Slime backend: use-case-independent rollout payload + +## Summary + +Today `_sample_to_payload` in the slime integration hardcodes the +math-agent shape: it emits `payload["prompt"]` and `payload["answer"]` +(from `sample.prompt` / `sample.label`) plus a nested +`payload["metadata"]` for anything else. That fits GSM8K but not the +three other in-tree examples (appworld needs `task_id`; migration +needs `repo_uri`/`metadata_uri`/...; officebench needs +`task_uri`/`testbed_uri`). Each has a different top-level payload +shape, and there's no way to express that through the current +pipeline. + +This plan changes one thing: **the JSONL row's `metadata` dict becomes +the agent payload verbatim**. No top-level key injection, no nesting. +Each agent declares whatever payload shape it wants by choosing what +keys to put in `metadata`. + +No new abstractions. No adapter class. No new CLI flag. Just one +simplification in `_sample_to_payload` and matching JSONL shape +examples in the docs. + +### Changes at a glance + +| Today | Proposed | +|---|---| +| `_sample_to_payload` builds payload from `sample.prompt`, `sample.label`, `sample.metadata`, plus a fall-through copy of Sample fields | `_sample_to_payload` returns `dict(sample.metadata)` — that's it | +| Payload structure: `{"prompt": ..., "answer": ..., "metadata": {...}, "_rollout": {...}}` | Payload structure: `{**metadata, "_rollout": {...}}` — agent-defined keys at the top level | +| Agents read `payload["prompt"]`, `payload["answer"]`, `payload["metadata"]["..."]` (nested) | Agents read `payload[""]` directly | +| JSONL shape: `{prompt, label}` hardcoded for math | JSONL shape: `{prompt, metadata: {}}` uniform across agents | + +## Why + +**The four in-tree agents want four different payload shapes.** Each +already parses its own `InvocationRequest` Pydantic model, or reads +specific keys like `task_id`. The current slime integration only +delivers `prompt` + `answer` + a bag-of-metadata at the top level, +which doesn't match: + +- **math** reads `payload["prompt"]`, `payload["answer"]` — fits current shape. +- **appworld** reads `payload["task_id"]` — needs `task_id` at the top level. +- **migration** does `InvocationRequest(**payload)` — needs `prompt`, `repo_uri`, `metadata_uri`, `require_maximal_migration`, `use_dependency_search_tool`, `apply_static_update` all at the top. +- **officebench** does `InvocationRequest(**payload)` — needs `task_uri`, `testbed_uri` at the top. + +Three of four agents can't run today without bespoke data-prep +changes or framework edits. + +**The clean fix is to let users own the payload shape.** slime's +Dataset already threads arbitrary dicts through to `Sample.metadata`. +The rollout function just needs to hand that dict to the agent +verbatim. The data author chooses the schema; the framework doesn't +have an opinion. + +**Why not keep metadata nested under `payload["metadata"]`?** That's +what the current code does, and it forces every agent to add a +`.get("metadata", {})` hop that serves no purpose. Worse, migration +and officebench use `InvocationRequest(**payload)` — they'd need +`InvocationRequest(**payload["metadata"])`, which breaks the Pydantic +model's self-documentation. Flattening to top-level matches how +agents already want to read the payload. + +**Why not drop `prompt` as a top-level JSONL key?** slime uses +`Sample.prompt` for tokenization, length filtering, and SGLang seed +tokens. It has to be a real string at the JSONL top level for slime's +Dataset. If the agent's actual input also happens to be a prompt +string (math, migration), the data author duplicates it inside +`metadata`. That's a small cost for a major simplification. + +### What happens when `prompt` appears in both? + +slime reads the two fields on different sides of the data layer, so +they never collide: + +| Field | Read by | Used for | +|---|---|---| +| top-level `prompt` | slime's Dataset via `prompt_key` | slime's tokenizer (length filter), chat template application, SGLang seed tokens | +| `metadata.prompt` | agent via `payload["prompt"]` | whatever the agent does with it | + +**Consequence:** you can legitimately set them to different values — +e.g. a short sentinel top-level `prompt` to minimize slime's length +filter, and a long templated instruction in `metadata.prompt`. Or +identical strings (what the data-prep recipe does for simplicity). +Nothing forces them to match. + +**Practical footgun:** if a user duplicates the prompt and later edits +only one, the two silently diverge — the agent sees one string and +slime's length filter sees another. Editorially fragile. The data-prep +recipe in SETUP.md will show a pattern that writes both from the same +source variable to avoid this. + +## Post-change JSONL shapes and agent code + +The slime-visible field (`prompt`) and the agent-visible payload +(`metadata` contents) are deliberately separate. slime gets a prompt +for its tokenizer; the agent gets whatever it wants. + +### math + +```jsonl +{"prompt": "Natalia sold clips to 48 friends...", + "metadata": {"prompt": "Natalia sold clips to 48 friends...", "answer": "72"}} +``` + +Agent (`rl_app.py`): + +```python +user_input = payload["prompt"] +answer = payload["answer"] +``` + +### appworld + +```jsonl +{"prompt": "AppWorld task seed", + "metadata": {"task_id": "82e20b2_1"}} +``` + +Agent: + +```python +task_id = payload["task_id"] +``` + +(The top-level `"prompt"` field is a throwaway string slime tokenizes +for length filtering. AppWorld's agent doesn't read it.) + +### migration + +```jsonl +{"prompt": "Migrate the Java project at {repo_path}...", + "metadata": { + "prompt": "Migrate the Java project at {repo_path}...", + "repo_uri": "s3://migration-bench/repos/abc.tar.gz", + "metadata_uri": "s3://migration-bench/meta/abc.json", + "require_maximal_migration": false, + "use_dependency_search_tool": false, + "apply_static_update": false + }} +``` + +Agent: + +```python +request = InvocationRequest(**payload) # unchanged from today +``` + +### officebench + +```jsonl +{"prompt": "OfficeBench task seed", + "metadata": { + "task_uri": "s3://bucket/officebench/1-1/config.json", + "testbed_uri": "s3://bucket/officebench/1-1/testbed.tar.gz" + }} +``` + +Agent: + +```python +request = InvocationRequest(**payload) # unchanged from today +``` + +## Design + +### The one code change + +`src/agentcore_rl_toolkit/backends/slime/integration/rollout.py::_sample_to_payload` +collapses to: + +```python +def _sample_to_payload(sample) -> dict: + """The agent payload is the JSONL row's `metadata` dict, verbatim. + + slime's Dataset reads the JSONL row's `metadata` field into + `Sample.metadata`; we pass it to the agent unchanged. The JSONL's + top-level `prompt` field is for slime (tokenization, length + filtering); the agent's payload shape is entirely defined by + whatever the data author put in `metadata`. + """ + if hasattr(sample, "metadata") and isinstance(sample.metadata, dict): + return dict(sample.metadata) # shallow copy to isolate from Sample state + return {} +``` + +Downstream (in `_process_one_episode`), the RolloutLauncher still +injects `_rollout` / `rollout_config` into the payload as today. The +final wire-format is: + +```python +{**metadata, "_rollout": {base_url, model_id, sampling_params, ...}} +``` + +### What goes away + +- The `sample.prompt → payload["prompt"]` injection. +- The `sample.label → payload["answer"]` injection (math-specific). +- The fall-through `to_dict()` loop that copies unfiltered Sample + fields into the payload — which was carrying silent leakage risk + (e.g. future slime Sample fields landing in agent payloads). +- The nested `payload["metadata"]` wrapping. `metadata` *is* the + payload now. + +### JSONL prep script + +SETUP.md's 3.2 prep snippet updates from: + +```python +with open(out, "w") as f: + for i, row in enumerate(ds): + if i >= 64: break + answer = row['answer'].split('####')[-1].strip() + f.write(json.dumps({'prompt': row['question'], 'label': answer}) + '\n') +``` + +to: + +```python +with open(out, "w") as f: + for i, row in enumerate(ds): + if i >= 64: break + answer = row['answer'].split('####')[-1].strip() + f.write(json.dumps({ + 'prompt': row['question'], # slime + 'metadata': {'prompt': row['question'], 'answer': answer}, # agent payload + }) + '\n') +``` + +### train.sh flag + +slime's `--label-key` is no longer needed (nothing consumes +`Sample.label`). Remove from `train.sh`. `--input-key prompt` stays — +slime still tokenizes the top-level prompt. + +## Downstream impact + +### math agent + +**Breaks** unless JSONL is regenerated. The existing +`data/gsm8k_tiny.jsonl` uses `{prompt, label}`; that has no +`metadata` → `Sample.metadata` is empty → payload is empty. Fix: the +prep script update above. + +Agent code change: `payload.get("answer")` → `payload["answer"]` +(or no change if `.get` is retained; it still works). + +### appworld agent + +Already reads `payload["task_id"]`. Needs its JSONL authored with +`{prompt, metadata: {task_id}}`. Zero code change in `rl_app.py`. + +### migration agent + +Already does `InvocationRequest(**payload)`. Works unchanged once +JSONL is authored as `{prompt, metadata: {}}`. + +### officebench agent + +Same as migration — works unchanged with the new JSONL shape. + +### SlimeRunner + +No change needed — it just ships `--input-key prompt` and +`--metadata-key metadata` (the latter is already slime's default). + +### rllm / verl + +No change. + +## Effect on slime scheduling and Megatron training + +A natural worry: if we move the agent's "real" prompt into metadata, +does slime get confused about what to tokenize, how to batch, or +which samples are too long? Investigation says the effect is +**narrow and bounded**. + +### Slime-side scheduling: mostly unaffected + +`Sample.prompt` is read by slime in three places; only one affects +scheduling: + +| Reader | Affects scheduling? | Notes | +|---|---|---| +| `filter_long_prompt` at Dataset load | **Yes** — filters rows whose tokenized `prompt` exceeds `rollout_max_prompt_len` | Happens once at dataset init, before rollout begins. Short top-level `prompt` → no rows filtered. | +| `slime.rollout.sglang_rollout` (slime's built-in rollout fn) | N/A | We replaced this with our `generate_rollout`; code path is dead on our side. | +| Logging (first-rollout / finish-rollout / rm_hub) | No | Pretty-printing only; no scheduling or training effect. | + +Rollout ordering, batching, SGLang engine routing, and concurrency are +all driven by dataset **index**, not prompt content. `data_source.py:: +get_samples` does `self.dataset.samples[offset : offset + N]` and then +deep-copies each row `n_samples_per_prompt` times. Prompt text has +zero influence on this path. + +**Consequence for the data-contract change:** + +- If the top-level `prompt` is short (e.g. a sentinel for appworld/ + officebench), length filtering is effectively disabled — every row + passes. Safe. +- If the top-level `prompt` is the real instruction (math, migration), + length filtering works as today — rows whose tokenized instruction + exceeds the context budget are dropped before rollout. +- If the *agent's* true prompt is long and lives only in `metadata`, + slime can't see its length. SGLang may then truncate at inference + time. Not broken, but less clean than pre-filtering. Authors who + want pre-filtering should put the real prompt at the top level too + (the `prompt` appears twice pattern). + +### Per-turn Samples passed to Megatron: unaffected + +Megatron trains on per-turn `Sample` objects built by `trace_to_sample` +in `integration/traces.py`, not on the dataset rows. Those Samples +have their own freshly-constructed fields: + +| Sample field | Source | Used by Megatron? | +|---|---|---| +| `tokens` | `TraceRecord.prompt_token_ids + completion_token_ids` (live from gateway) | **Yes** — the tokens trained on | +| `response_length` | `len(completion_token_ids)` | Yes | +| `loss_mask` | `[1] * response_length` | Yes | +| `rollout_log_probs` | `TraceRecord.logprobs` | Yes (off-policy correction) | +| `reward` | `extract_reward(result)` — from agent's S3 output | Yes | +| `group_index`, `index`, `session_id` | bookkeeping | Yes (grouping, logging) | +| `metadata` | `{"task_index", "gateway_session_id", "turn_index"}` + `"task_metadata"` (copied from dataset row) | Only two keys read by Megatron: `raw_reward`, `round_number` (neither set by our path) | +| `prompt`, `label` | Copied from dataset row for logging/traceability only | **No** — Megatron never reads these | + +**Megatron's training tensors are synthesized entirely from +gateway-captured traces.** The dataset row's contents — `prompt`, +`metadata`, whatever — do not flow into `tokens`, `loss_mask`, or +gradients. They only affect: + +1. What the agent receives as its payload (the primary behavior change). +2. What gets preserved in `Sample.metadata["task_metadata"]` for + traceability / logging. + +### One non-functional nuance: per-turn Sample.prompt / label become less useful for logging + +Today, `_process_one_episode` copies the dataset-row's `prompt` and +`label` onto each per-turn Sample (lines 323-325 of `rollout.py`) for +traceability: + +```python +for s in samples: + s.prompt = sample.prompt + s.label = sample.label + if sample.metadata: + s.metadata["task_metadata"] = sample.metadata +``` + +After the data-contract change: +- `sample.prompt` will be whatever the data author put at the JSONL + top level (potentially a short sentinel). +- `sample.label` will typically be `None` (the `label_key` CLI arg is + dropped). + +So slime's first-rollout / finish-rollout log lines (which display +`sample.prompt` and `sample.label`) will show the sentinel instead of +the real prompt, and no label. The **real** agent input is preserved +in `sample.metadata["task_metadata"]` — logs that want to show it can +read from there. + +This is cosmetic, not functional. No gradients, rewards, or +scheduling decisions depend on the affected fields. + +## Backwards compatibility + +**Not backward compatible for math's existing JSONL shape.** The +current `{prompt, label}` rows silently become empty-metadata rows +under the new rule, and the agent receives `{"_rollout": {...}}` with +no `prompt`/`answer`. Fix is trivial (regenerate the JSONL with the +new prep script), but it's a breaking change that ships alongside the +code change. + +Options: + +1. **Break cleanly** — ship the code change + prep-script update + together as a minor version bump; users regenerate JSONLs. +2. **Parallel-support period** — keep the old `prompt`/`label` injection + as a fallback when `sample.metadata` is empty. Adds ~6 lines of + back-compat code; we'd remove it when cleaning up the other 0.2 + changes. + +Recommend (2) for one release, then remove in the next minor. + +## Out of scope + +- **Changing how agents receive `_rollout` / `rollout_config`.** That's + the RolloutLauncher's contract, unrelated to data shape. Stays as-is. +- **Supporting `prompt` as a list of chat messages.** slime has + `apply_chat_template=True` for that; out of scope for this plan. +- **Auto-deriving the slime `prompt` from metadata.** Requires a + slime-side hook. Not worth the lift; data authors duplicate the + string. +- **Validating agent payload shape against a schema.** Each agent + already does its own validation (Pydantic, direct key access). + No new framework layer. +- **Renaming the JSONL top-level `metadata` key.** slime's default is + `"metadata"`; we'd have to plumb a rename through slime's CLI to + change it. Not worth the churn. + +## Open questions + +1. **Hard break vs. parallel-support period?** Recommend parallel + support for one release; log a deprecation warning when + `sample.metadata` is empty and `sample.prompt`/`sample.label` are + non-empty (the old math shape). +2. **Do we remove `_label_key` from `train.sh`?** Yes, with the + JSONL change. Keeping it costs nothing but adds noise. +3. **Should SETUP.md show all four JSONL shapes or just math?** + Recommend all four — it anchors the agent-agnostic narrative. + +## Task checklist (single PR, 0.2.0) + +**Core** + +- [ ] `integration/rollout.py::_sample_to_payload`: collapse to the + 3-line `dict(sample.metadata)` version. Delete the + prompt/label/to_dict fall-through logic. +- [ ] Optional: add a one-release-only fallback that logs a + `DeprecationWarning` and injects `prompt`/`answer` when + `sample.metadata` is empty. + +**Examples** + +- [ ] Regenerate `examples/strands_math_agent/data/gsm8k_tiny.jsonl` + (and equivalent scripts) with the `{prompt, metadata: {...}}` + shape. +- [ ] Update math `rl_app.py` to read `payload["prompt"]`, + `payload["answer"]` (no change, but verify). +- [ ] Update appworld `rl_app.py` — already reads + `payload["task_id"]`; verify with a sample JSONL. +- [ ] migration `rl_app.py` — `InvocationRequest(**payload)` + unchanged; verify. +- [ ] officebench `rl_app.py` — same. +- [ ] Add JSONL sample files to all four `examples/` dirs showing + the expected shape. + +**Docs** + +- [ ] SETUP.md 3.2: update the data-prep snippet to the new shape, + show one non-math example (e.g. migration) as a side-by-side + so the agent-agnostic nature is obvious. +- [ ] Regenerate API reference. + +**Tests** + +- [ ] Unit: `_sample_to_payload(Sample(metadata={"task_id": "x"}))` + → `{"task_id": "x"}`. +- [ ] Unit: `_sample_to_payload(Sample(metadata={}))` → `{}`. +- [ ] Unit: `_sample_to_payload(Sample())` (no metadata attr) → `{}`. + +**Release** + +- [ ] Ship alongside the core-api-rename as 0.2.0 (both are + breaking; consolidate the user-migration story). + +## Milestones + +1. **M1** — update `_sample_to_payload`, regenerate math JSONL, run + 3B smoke. Half day. +2. **M2** — update 3 other example JSONLs + SETUP.md. Half day. +3. **M3** — PR review. Calendar. + +--- + +## Appendix A — Identity and `group_index` in slime + +This section is background investigation, not part of the proposed +change. It explains how slime assigns identities to Samples today, +so readers can reason about whether metadata-derived `input_id`s +(content hashes, row offsets, etc.) are needed. **Conclusion: not +needed for the data-contract change itself, but useful context if +cross-run identity ever becomes a concrete need.** + +### The three IDs in play + +``` +result_key = f"{exp_id}/{input_id}/{session_id}.json" + ^^^^^^^^ ^^^^^^^^^ ^^^^^^^^^^^^ + run which row which rollout attempt + in dataset of that row +``` + +- `exp_id` — user-chosen run name. Propagated via `SlimeRunner(exp_id=...)`. +- `session_id` — per-attempt UUID from `uuid.uuid4()` in `integration/rollout.py`. +- `input_id` — **currently set equal to `session_id`** in our code + (`integration/rollout.py:294`), so the "which row" level is + effectively unused; each attempt looks unique. + +### How slime assigns per-Sample identity + +`slime.rollout.data_source.PromptDataset` maintains two persistent +counters: + +```python +self.sample_group_index = 0 # monotonic across the whole run +self.sample_index = 0 # monotonic across the whole run +``` + +`get_samples(N)` — called once per rollout step to produce N prompts' +worth of training samples — does: + +```python +prompt_samples = self.dataset.samples[offset : offset + N] + +samples = [] +for prompt_sample in prompt_samples: + group = [] + for _ in range(args.n_samples_per_prompt): # fan out K copies per row + sample = copy.deepcopy(prompt_sample) + sample.group_index = self.sample_group_index # shared by all K copies + sample.index = self.sample_index # unique per copy + self.sample_index += 1 + group.append(sample) + self.sample_group_index += 1 # next row gets next group_index + samples.append(group) +``` + +Both counters are **checkpointed** in `save()` / loaded in +`load_state_dict()`, so resuming a job keeps the numbering monotonic. + +### Concrete numbering example + +`rollout_batch_size=32`, `n_samples_per_prompt=8`, JSONL has 64 rows: + +| Rollout step | JSONL rows consumed | `group_index` range | `sample.index` range | +|---|---|---|---| +| 0 | rows 0..31 | 0..31 | 0..255 (32 × 8) | +| 1 | rows 32..63 | 32..63 | 256..511 | +| 2 (epoch wraps) | rows 0..31 (epoch 1) | 64..95 | 512..767 | +| 3 | rows 32..63 | 96..127 | 768..1023 | + +Key property: after the epoch wrap, the **same dataset row gets a +different `group_index`** than its first visit. `group_index` is an +identity of "this *visit* to the row," not "identity of the row +itself." + +### What `group_index` is used for + +`slime.ray.rollout.py:1248`: + +```python +all_sample_groups = group_by(all_samples, lambda s: s.group_index) +``` + +This is the **GRPO group boundary**. All Samples sharing a +`group_index` are the K rollouts of the same prompt-visit, and GRPO +mean-centers their rewards against each other. So `group_index` +serves *within-rollout-step* identity — it's exactly what GRPO needs +and nothing more. + +### Why this doesn't drive the data-contract change + +The data-contract proposal only changes **what goes into +`payload["metadata"]`** and what the agent reads. It doesn't touch +how slime assigns `group_index`, `sample.index`, or what slime +records in `metadata["task_metadata"]`. All three counters continue +to work identically. + +### Where content-derived IDs would matter (future work, not this plan) + +If a future need arises for: + +- **Cross-run row identity** ("did exp-124 process the same row-42 + content that exp-123 did?") +- **Cross-epoch row identity** (`group_index` visits differ) +- **Cross-dataset correlation** ("which samples in eval-v2 test the + same inputs as eval-v1?") + +…then a content hash of `sample.metadata` would give a stable ID: + +```python +import hashlib, json +input_id = hashlib.sha256( + json.dumps(sample.metadata, sort_keys=True).encode() +).hexdigest()[:16] +``` + +The groundwork is laid by the data-contract change — metadata becomes +the agent payload and the authoritative per-row state — so a +content-hash `input_id` becomes trivially implementable later. But it +is explicitly deferred: the current plan preserves +`input_id == session_id` behavior. + +### Non-content row identity: `group_index` as a quick win + +If someone wants "find all attempts of the same row within a run" +without cross-run stability, swapping: + +```python +# in integration/rollout.py::_process_one_episode +input_id=session_id +# → +input_id=f"g{sample.group_index}" +``` + +…gives S3 keys like `exp-123/g42/abcd.json` where all `n_samples_per_prompt` +attempts of group 42 sort together. Also deferred; just noting the +5-line version exists. diff --git a/docs/roadmap/committed/slime-runner-entrypoint.md b/docs/roadmap/committed/slime-runner-entrypoint.md new file mode 100644 index 0000000..51aadbc --- /dev/null +++ b/docs/roadmap/committed/slime-runner-entrypoint.md @@ -0,0 +1,415 @@ +--- +title: "SlimeRunner: one Python entry point for slime-backed training" +description: "Wrap train.sh + config.yaml behind a single Python class so users write `SlimeRunner(...).train()` instead of editing 121 lines of bash + YAML." +--- +# SlimeRunner: one Python entry point for slime-backed training + +## Summary + +Today a slime-backend training run requires editing two files: +- `train.sh` (121 lines of bash, Ray/SGLang plumbing, 50+ slime CLI flags) +- `config.yaml` (ACR ARN, bucket, sampling settings) + +Plus setting 4-5 env vars. Users who want to vary the model or dataset +either copy-edit train.sh or export environment variables for every +knob. This plan replaces the train.sh+config.yaml+env-vars combo with +a single Python class, `SlimeRunner`, whose constructor takes the +handful of fields that actually change per experiment and hides the +rest behind defaults. Under the hood it shells out to `ray job submit` +the same way `train.sh` does today. + +**This is an additive convenience layer.** `train.sh` stays in the +repo (unchanged) as the low-level escape hatch for users who need to +customize what the class hides. + +### Before / after + +**Before (user edits train.sh + config.yaml, runs bash):** +```bash +# config.yaml: edit 8 fields +# train.sh: set env vars or edit defaults +export SLIME_DIR=/root/slime MEGATRON_DIR=/root/Megatron-LM \ + MODEL_DIR=/path/to/Qwen2.5-3B-Instruct \ + DATA_PATH=/path/to/gsm8k_tiny.jsonl +bash train.sh +``` + +**After (user writes one Python file):** +```python +# examples/math_agent/train.py +from agentcore_rl_toolkit.backends.slime import SlimeRunner + +SlimeRunner( + exp_id="gsm8k-3b-smoke", + agent_runtime_arn="arn:aws:bedrock-agentcore:...", + s3_bucket="my-bucket", + model_dir="/path/to/Qwen2.5-3B-Instruct", + data_path="/path/to/gsm8k_tiny.jsonl", + model_type="qwen2.5-3B", +).train(num_rollout=1) +``` + +Runs identically; same Ray cluster, same SGLang engines, same GRPO. + +## Why + +**train.sh is intimidating and viral.** 121 lines of bash with ~50 flag +args, many of which are cargo-culted from slime's own reference +scripts. Users who want to change one thing end up reading all of it. + +**config.yaml vs env-var vs train.sh-arg is a three-way split with no +good rule.** A user today has to know: +- ACR ARN goes in config.yaml +- MODEL_DIR goes in env var +- `--num-rollout` goes in train.sh `--num-rollout` flag directly +- `--optimizer-cpu-offload` lives inside train.sh's CLI and can't be + overridden from outside + +A single Python constructor gives one place for everything and lets +us pick sensible defaults per knob without surfacing the decision. + +**The knobs that actually vary per experiment are few.** Walking +through train.sh: + +| Varies per experiment | Fixed in practice | +|---|---| +| `agent_runtime_arn`, `s3_bucket` | `gateway_port=9090` | +| `model_dir`, `data_path`, `model_type` | `acr_timeout=900`, `acr_tps_limit=25`, `max_concurrent=100` | +| `num_rollout` | `rollout_batch_size=32`, `n_samples_per_prompt=8` | +| `num_gpus`, `tp_size` | `lr=1e-6`, `weight_decay=0.1`, `adam_beta2=0.98` | +| `model_id` (rare) | `reward_postprocessing="grpo"` | +| | `--optimizer-cpu-offload` etc. (always set) | + +Five-ish fields are genuinely per-experiment; the rest are "you'd +change them only if you know what you're doing." Natural home for +defaults-in-code + kwargs-for-escape. + +**train.sh's hidden complexity is learnable once, not every run.** +The Ray start/stop, `--runtime-env-json` construction, `source +scripts/models/qwen2.5-3B.sh`, `--norm-epsilon` override — these are +setup steps that should happen once per training framework version, +not once per user. + +## Post-change usage + +**Minimal (3B smoke test):** + +```python +SlimeRunner( + exp_id="gsm8k-3b-smoke", + agent_runtime_arn="arn:aws:bedrock-agentcore:us-west-2:...:runtime/...", + s3_bucket="my-bucket", + model_dir="/workspace/slime_workdir/models/Qwen2.5-3B-Instruct", + data_path="/workspace/slime_workdir/data/gsm8k_tiny.jsonl", + model_type="qwen2.5-3B", +).train() # num_rollout=1 by default (smoke test) +``` + +**32B full run:** + +```python +SlimeRunner( + exp_id="gsm8k-32b-grpo-2026-05-10", + agent_runtime_arn="...", + s3_bucket="...", + model_dir="/workspace/slime_workdir/models/Qwen2.5-32B-Instruct", + data_path="/workspace/slime_workdir/data/gsm8k_tiny.jsonl", + model_type="qwen2.5-32B", + num_gpus=8, + tp_size=8, + rollout_gpus_per_engine=8, +).train(num_rollout=100) +``` + +**Power user who needs to override a hidden default:** + +```python +SlimeRunner( + exp_id="gsm8k-3b-lr-sweep-5e7", + agent_runtime_arn="...", + s3_bucket="...", + model_dir="...", + data_path="...", + model_type="qwen2.5-3B", + # Override defaults; kwarg names match train.sh's flags + n_samples_per_prompt=16, + lr=5e-7, + eps_clip_high=0.3, +).train(num_rollout=50) +``` + +**Native slime arg passthrough.** Any CLI flag slime or Megatron-LM +accepts can be passed through the runner using its snake-case form. +The runner converts `foo_bar=value` → `--foo-bar value` when building +the slime CLI, matching how slime itself maps argparse. If you want to +set a slime arg we didn't surface as a named kwarg, use `extra_flags`: + +```python +SlimeRunner( + ..., + extra_flags=["--num-epoch", "3", "--use-rollout-routing-replay"], +).train() +``` + +Reference for the full set of accepted flags: +- **slime arguments:** [`slime/utils/arguments.py`](https://github.com/THUDM/slime/blob/main/slime/utils/arguments.py) — RL / rollout / training knobs. +- **Megatron-LM arguments:** [`megatron/training/arguments.py`](https://github.com/NVIDIA/Megatron-LM/blob/main/megatron/training/arguments.py) — model-parallelism, optimizer, checkpointing. +- **SGLang server args:** [`sglang.srt.server_args`](https://github.com/sgl-project/sglang/blob/main/python/sglang/srt/server_args.py) — inference-engine tuning; exposed by slime via `--sglang-*` flags. + +Everything listed in those three files is reachable from `SlimeRunner` +without framework changes. + +## Design + +### Module layout and public surface + +``` +src/agentcore_rl_toolkit/backends/slime/ +├── __init__.py # exposes only `SlimeRunner` +├── runner.py # SlimeRunner (new) +├── SETUP.md +├── integration/ # implementation detail — not part of public surface +│ ├── __init__.py +│ ├── gateway.py +│ ├── rewards.py +│ ├── rollout.py # generate_rollout still referenced by slime via dotted path +│ └── traces.py +├── patches/ # SGLang patch scripts +└── examples/ + └── math_agent/ +``` + +**Public API:** +```python +from agentcore_rl_toolkit.backends.slime import SlimeRunner # only this +``` + +`slime/__init__.py`: +```python +from .runner import SlimeRunner + +__all__ = ["SlimeRunner"] +``` + +Everything in `integration/` remains importable by its full path — +that's load-bearing because `train.sh` passes +`agentcore_rl_toolkit.backends.slime.integration.rollout.generate_rollout` +as a slime CLI arg, and slime loads it via `importlib`. But those +modules are **not** part of the documented public API — users shouldn't +import them directly. The `__init__` re-export is the contract; dotted +paths to `integration.*` are internal plumbing. + +No renames, no file moves — `integration/` is already organized +correctly. + +### Public class + +```python +# src/agentcore_rl_toolkit/backends/slime/runner.py + +from dataclasses import dataclass, field +from pathlib import Path + +@dataclass +class SlimeRunner: + # --- Required: per-experiment, no sensible default --- + exp_id: str # experiment name; used as S3 prefix and wandb run group + agent_runtime_arn: str + s3_bucket: str + model_dir: str # HF checkpoint path (used for both --hf-checkpoint and --ref-load) + data_path: str # training JSONL + model_type: str # e.g. "qwen2.5-3B" — selects slime model script + + # --- Optional: cluster --- + num_gpus: int = 8 + tp_size: int = 2 + rollout_gpus_per_engine: int = 2 + slime_dir: str = "/root/slime" # container default + megatron_dir: str = "/root/Megatron-LM" # container default + + # --- Optional: ACR / toolkit --- + model_id: str = "default" + acr_timeout: int = 900 + acr_tps_limit: int = 25 + max_concurrent: int = 100 + gateway_port: int = 9090 + reward_postprocessing: str = "grpo" + + # --- Optional: training hyperparameters --- + rollout_batch_size: int = 32 + n_samples_per_prompt: int = 8 + rollout_max_response_len: int = 1024 + rollout_temperature: float = 1.0 + lr: float = 1e-6 + eps_clip: float = 0.2 + eps_clip_high: float = 0.28 + weight_decay: float = 0.1 + adam_beta2: float = 0.98 + sglang_mem_fraction_static: float = 0.7 + max_tokens_per_gpu: int = 9216 + + # --- Escape hatch --- + extra_flags: list[str] = field(default_factory=list) + + # --- Methods --- + def train(self, num_rollout: int = 1) -> None: + """Run the training job. Blocks until the slime job exits.""" + self._stop_stale_processes() + self._start_ray() + model_args = self._source_model_script() + runtime_env = self._build_runtime_env() + self._submit_ray_job(num_rollout, model_args, runtime_env) + + # Internal helpers (_stop_stale_processes, _start_ray, _source_model_script, + # _build_runtime_env, _submit_ray_job) mirror train.sh's current bash steps, + # one-to-one. +``` + +All kwargs default; users only pass what varies from the defaults. + +### How it shells out + +`.train()` uses `subprocess` to reproduce train.sh's current behavior +step-by-step: + +1. `pkill -9 sglang || true; ray stop --force || true; sleep 3` +2. `ray start --head --num-gpus ${num_gpus} --disable-usage-stats` +3. `source ${slime_dir}/scripts/models/${model_type}.sh` — captured via + `subprocess.check_output("bash -c '...; printf \"%s\\0\" \"${MODEL_ARGS[@]}\"'")`, + split on null bytes into a Python list. +4. Build `runtime_env_json` dict in Python (we already have the + python-in-bash snippet in train.sh; becomes native here). +5. `ray job submit --address http://127.0.0.1:8265 --runtime-env-json=... -- python3 ${slime_dir}/train.py ` +6. Stream stdout/stderr to the parent process; non-zero exit raises. + +No new Ray bindings, no new slime imports. If slime/Ray changes their +interface tomorrow, one subprocess command changes, not a refactor. + +### config.yaml fate + +`config.yaml` becomes **optional**. Users who prefer a config file can +continue to use it: + +```python +SlimeRunner.from_yaml("config.yaml").train() +``` + +`from_yaml` is a 10-line classmethod that reads the file and passes +the dict to `SlimeRunner(**kwargs)`. Keeps the old path working for +anyone already using yaml. The class is the primary surface; yaml is +a convenience loader. + +### train.sh fate + +**Stays in the repo, unchanged.** Two reasons: + +1. Users who need to customize something the class doesn't expose + (e.g. debugging slime flags) can still drop to bash. +2. It serves as the executable reference for what the class has to + replicate — we can diff it against the class's subprocess commands + to catch drift. + +The example dir ships `train.py` (using `SlimeRunner`) as the +primary entry point; `train.sh` as the "advanced / debugging" option. + +## Downstream impact + +### Math agent example + +`examples/math_agent/` gains a `train.py`; existing `train.sh` / +`config.yaml` stay as-is. Users get pointed at train.py first; SETUP.md +is updated to show the 5-line Python snippet. + +### New agents (hypothetical) + +Instantiating `SlimeRunner` with a different `data_path` and +`agent_runtime_arn` is all they need. No copying train.sh. + +### rllm / verl / out-of-tree backends + +Untouched. This is a slime-backend convenience — rllm and verl have +their own entry points. + +### Slime internals + +No changes — we call slime through the same `ray job submit` CLI as +before. + +## Backwards compatibility + +**Fully backwards compatible.** train.sh and config.yaml are +unchanged. Adding the class doesn't remove anything. + +## Out of scope + +- **Replacing train.sh.** It stays as the escape hatch. We could + remove it later if usage drops to zero, but not in this PR. +- **Live log streaming / progress bars.** `.train()` blocks and + streams raw stdout; no UI work. +- **Distributed / multi-node support.** Current train.sh is + single-node; the class mirrors that. +- **Changing the `RolloutLauncher`, `RewardFunction`, or payload + contract.** Pure subprocess wrapper. + +## Open questions + +1. **Should `train()` be sync (blocks) or async / detached?** Recommend + sync — matches what `bash train.sh` does today. Async can come + later if users ask for it. + +2. **Return value from `train()`.** None (fire-and-forget like bash + today) vs. a results dict (path to wandb run, final rollout + metrics, etc.). Recommend None for v1; add later if needed. + +3. **Should hyperparameters be a nested `HyperParams` object + (`SlimeRunner(..., hyper=HyperParams(lr=1e-6))`) or flat kwargs as + proposed?** Recommend flat kwargs — 11 fields is manageable, nesting + adds an import and a line of user code for small gain. + +4. **What about `config.yaml` for users who like YAML?** Kept via + `SlimeRunner.from_yaml(path)`. Not the primary path, but still + supported. + +## Task checklist (single PR) + +**Core** + +- [ ] Create `src/agentcore_rl_toolkit/backends/slime/runner.py` with + the `SlimeRunner` dataclass + `.train()` that shells out. +- [ ] Update `src/agentcore_rl_toolkit/backends/slime/__init__.py` to + `from .runner import SlimeRunner` and set `__all__ = ["SlimeRunner"]`. +- [ ] Leave `integration/` and `patches/` as they are — same files, + same dotted paths. No re-exports from the slime package root. +- [ ] Add `SlimeRunner.from_yaml(path)` classmethod. + +**Example** + +- [ ] `examples/math_agent/train.py`: 8-line Python replacement for + the common case. +- [ ] Leave `examples/math_agent/train.sh` + `config.yaml` untouched. + +**Docs** + +- [ ] SETUP.md 3.4: show `train.py` as the primary recipe; keep + train.sh example as "advanced / debugging." +- [ ] Regenerate API reference with the new class. + +**Tests** + +- [ ] Unit: `SlimeRunner(**minimum_kwargs)._build_runtime_env()` + produces the expected JSON (mirrors train.sh's python snippet). +- [ ] Unit: `SlimeRunner.from_yaml` round-trips sample config.yaml + → equivalent kwargs. +- [ ] Integration smoke (manual, not CI): 3B smoke test via `train.py` + → same rollout 0 metrics as today's train.sh. + +**Release** + +- [ ] Minor version bump, additive. + +## Milestones + +1. **M1** — draft the class, run a local smoke to confirm the + shell-out produces a working rollout. 1 day. +2. **M2** — PR with tests + docs. Half day. +3. **M3** — land. From 0d9d29f21d33da35ebf9a686276910ede5ed2177 Mon Sep 17 00:00:00 2001 From: Panpan Xu Date: Wed, 6 May 2026 00:11:03 +0000 Subject: [PATCH 3/5] docs(roadmap): rename rollout_server.py to rollout_executor.py in core-api-rename plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AgentCoreRLApp is not a multi-request server — the ACR runtime loads the app and executes it once per session to produce a single rollout. "rollout_executor.py" captures that semantics better than "rollout_server.py", which implied a long-lived request handler. Co-Authored-By: Claude Opus 4.7 --- docs/roadmap/committed/core-api-rename.md | 32 +++++++++++++---------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/docs/roadmap/committed/core-api-rename.md b/docs/roadmap/committed/core-api-rename.md index 788db36..169eb3d 100644 --- a/docs/roadmap/committed/core-api-rename.md +++ b/docs/roadmap/committed/core-api-rename.md @@ -1,8 +1,8 @@ --- -title: "Core API rename: rollout-server / rollout-launcher / rollout-config" +title: "Core API rename: rollout-executor / rollout-launcher / rollout-config" description: "ART's public surface uses generic names (app.py, client.py," --- -# Core API rename: rollout-server / rollout-launcher / rollout-config +# Core API rename: rollout-executor / rollout-launcher / rollout-config ## Summary @@ -27,7 +27,7 @@ unchanged. | Today | Renamed | |---|---| -| `app.py` | `rollout_server.py` | +| `app.py` | `rollout_executor.py` | | `client.py` | `rollout_launcher.py` | | `reward_function.py` | `reward.py` | @@ -82,11 +82,14 @@ files. Coherence requires a single pass. > ART has two sides of one wire. > -> The **rollout side** runs on ACR: an `AgentCoreRLApp` with a -> `@rollout_entrypoint` handler (in `rollout_server.py`). It reads -> per-rollout settings from `payload["rollout_config"]` (`base_url`, -> `model_id`, `api_key`, `sampling_params`), runs the agent, -> computes a reward, returns a dict. Results save to S3. +> The **executor side** is the agent code packaged into an +> `AgentCoreRLApp` with a `@rollout_entrypoint` handler (in +> `rollout_executor.py`). It isn't a multi-request server — the ACR +> runtime loads the app and executes it once per session to produce +> one rollout. It reads per-rollout settings from +> `payload["rollout_config"]` (`base_url`, `model_id`, `api_key`, +> `sampling_params`), runs the agent, computes a reward, returns a +> dict. Results save to S3. > > The **launcher side** runs in your trainer or evaluator: a > `RolloutLauncher` submits rollouts and collects results (in @@ -101,7 +104,7 @@ files. Coherence requires a single pass. ``` __init__.py -rollout_server.py # AgentCoreRLApp +rollout_executor.py # AgentCoreRLApp rollout_launcher.py # RolloutLauncher, RolloutFuture, RolloutBatch, ... reward.py # RewardFunction backends/ @@ -109,9 +112,10 @@ frameworks/ ``` Class-level asymmetry (`App` vs `Launcher`) is kept deliberately: -the App subclasses an AWS-managed runtime; the Launcher is a plain -Python object. File-level symmetry (`rollout_server.py` vs -`rollout_launcher.py`) shows the pairing on `ls`. +the App subclasses an AWS-managed runtime and is *executed* per +session; the Launcher is a plain Python object that *submits* work. +File-level pairing (`rollout_executor.py` vs `rollout_launcher.py`) +shows the two sides on `ls`. ## Migration @@ -176,7 +180,7 @@ Not integrated in-tree or out-of-tree today. When integration work starts, the a ## Task checklist (single PR, 0.2.0) **Core renames** -- [ ] `git mv src/agentcore_rl_toolkit/app.py → rollout_server.py` +- [ ] `git mv src/agentcore_rl_toolkit/app.py → rollout_executor.py` (keeps `AgentCoreRLApp`). - [ ] `git mv src/agentcore_rl_toolkit/client.py → rollout_launcher.py` and rename `RolloutClient` → `RolloutLauncher`, @@ -220,7 +224,7 @@ Not integrated in-tree or out-of-tree today. When integration work starts, the a - [ ] New imports resolve (`from agentcore_rl_toolkit import RolloutLauncher`). - [ ] Old imports fail loudly (`ImportError`, not silent alias). -- [ ] `payload["rollout_config"]` round-trips launcher → server. +- [ ] `payload["rollout_config"]` round-trips launcher → executor. - [ ] Missing `api_key` raises a clear `KeyError`, not defaults to `"EMPTY"` silently. From 8cf16a329156b271dd09f464102d8933dd0a55ba Mon Sep 17 00:00:00 2001 From: Panpan Xu Date: Wed, 6 May 2026 00:11:03 +0000 Subject: [PATCH 4/5] docs(roadmap): require end-to-end smoke test before landing slime data-contract PR Unit tests pin the _sample_to_payload contract in isolation, but an integration bug would still hit the first user. Add a required pre-PR smoke-test gate to the slime-data-contract plan: regenerate the JSONL, run train.sh with NUM_ROLLOUT=10 on Qwen2.5-3B-Instruct, confirm rollout metrics and training steps, paste evidence in the PR body. Also adds a unit-test item for the shallow-copy invariant (mutations to the returned payload must not leak into Sample.metadata). Co-Authored-By: Claude Opus 4.7 --- docs/roadmap/committed/slime-data-contract.md | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/docs/roadmap/committed/slime-data-contract.md b/docs/roadmap/committed/slime-data-contract.md index cbe3e17..1023e50 100644 --- a/docs/roadmap/committed/slime-data-contract.md +++ b/docs/roadmap/committed/slime-data-contract.md @@ -450,6 +450,27 @@ Recommend (2) for one release, then remove in the next minor. → `{"task_id": "x"}`. - [ ] Unit: `_sample_to_payload(Sample(metadata={}))` → `{}`. - [ ] Unit: `_sample_to_payload(Sample())` (no metadata attr) → `{}`. +- [ ] Unit: returned dict is a shallow copy — mutating it does not + leak into `Sample.metadata`. + +**Required pre-PR smoke test** + +Unit tests cover the contract in isolation; they can't catch an +integration bug that would hit users on the first run. Before +opening the PR, run an end-to-end training smoke test against a +real ACR deployment and paste the evidence into the PR body: + +- [ ] Regenerate `gsm8k_tiny.jsonl` with the new `{prompt, metadata}` + shape. +- [ ] Run `bash train.sh` with `NUM_ROLLOUT=10` on Qwen2.5-3B-Instruct. +- [ ] Confirm: + - at least one rollout step completes (`rollout/raw_reward` is + non-trivial, i.e. > 0) + - `train/loss`, `train/grad_norm`, `train/step` appear in logs + - no `Exception`, `Traceback`, or `FAILED` session status +- [ ] Capture the rollout-0 metrics line + a few sample rewards in + the PR description so reviewers can see the contract worked + end-to-end. **Release** From 6cd781b47d5158e2898d16fe96c4e92789d8b7be Mon Sep 17 00:00:00 2001 From: Panpan Xu Date: Wed, 6 May 2026 00:11:04 +0000 Subject: [PATCH 5/5] docs(roadmap): move slime-data-contract plan to done/ Implemented in #61. Moves the plan out of committed/ (in-flight) into done/ (shipped), alongside the other completed plans. Co-Authored-By: Claude Opus 4.7 --- docs/roadmap/{committed => done}/slime-data-contract.md | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename docs/roadmap/{committed => done}/slime-data-contract.md (100%) diff --git a/docs/roadmap/committed/slime-data-contract.md b/docs/roadmap/done/slime-data-contract.md similarity index 100% rename from docs/roadmap/committed/slime-data-contract.md rename to docs/roadmap/done/slime-data-contract.md