Skip to content

feat(agentgym): EvoSwarm → AgentGym-RL → HuggingFace publisher pipeline - #935

Merged
POWERFULMOVES merged 5 commits into
mainfrom
feat/evoswarm-hf-publisher
Mar 15, 2026
Merged

feat(agentgym): EvoSwarm → AgentGym-RL → HuggingFace publisher pipeline#935
POWERFULMOVES merged 5 commits into
mainfrom
feat/evoswarm-hf-publisher

Conversation

@POWERFULMOVES

@POWERFULMOVES POWERFULMOVES commented Mar 15, 2026

Copy link
Copy Markdown
Owner

Summary

  • New NATS event chain: agentgym.train.completed.v1 → auto-publish to HF → agentgym.model.published.v1
  • AgentGym-RL coordinator gains HF publisher with repo creation and model upload
  • Evo controller wired with training completion callback and NATS event emission
  • Updated base model default to Qwen3-8B-Instruct
  • Triggers benchmark-viz pipeline on successful publish

Files Changed

  • pmoves/services/agentgym-rl-coordinator/app.py — HF publisher endpoint
  • pmoves/services/evo-controller/agentgym_integration.py — training completion callback
  • pmoves/env.agentgym.example — HF_TOKEN and HF_ORGANIZATION vars
  • .claude/context/nats-subjects.md — agentgym.* NATS subjects

Test plan

  • Verify AgentGym-RL coordinator starts with new HF publisher code
  • Confirm NATS subjects documented in context match implementation
  • Check env.agentgym.example has all required vars
  • Validate evo-controller callback signature matches coordinator expectations

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • New training lifecycle events (start, completed, model published) and auto-publish of trained models/datasets to HuggingFace; triggers benchmark visualization.
    • Added healthz and Prometheus-style metrics endpoints.
  • Updates

    • Default base model updated to Qwen/Qwen3-8B.
    • Events now use timezone-aware UTC timestamps.
    • New env vars for HuggingFace integration (HF_TOKEN, HF_ORG).
  • Bug Fixes

    • Trajectory ID validation and improved error handling/logging for publish flows.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai

coderabbitai Bot commented Mar 15, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@POWERFULMOVES has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 14 minutes and 12 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9a48ad84-509a-4bbb-9abd-5b988484c80b

📥 Commits

Reviewing files that changed from the base of the PR and between a3046cc and adef556.

📒 Files selected for processing (4)
  • .claude/context/nats-subjects.md
  • pmoves/env.agentgym.example
  • pmoves/services/agentgym-rl-coordinator/app.py
  • pmoves/services/evo-controller/agentgym_integration.py
📝 Walkthrough

Walkthrough

Adds AgentGym RL training lifecycle NATS subjects and handlers, HF auto-publishing from the RL coordinator, a training-completed publisher in Evo-Controller, new /healthz and /metrics endpoints, and environment variables/default base model updates for HuggingFace integration.

Changes

Cohort / File(s) Summary
Event Specs
.claude/context/nats-subjects.md
Adds three NATS subjects: agentgym.train.started.v1, agentgym.train.completed.v1, and agentgym.model.published.v1 with payload schemas, subscribers, and triggers.
Environment / Config
pmoves/env.agentgym.example
Updates AGENTGYM_BASE_MODEL to Qwen/Qwen3-8B; adds HF_TOKEN and HF_ORG env vars for HuggingFace publishing.
RL Coordinator Service
pmoves/services/agentgym-rl-coordinator/app.py
Adds handler for agentgym.train.completed.v1, JSON and UUID validation, HF auto-publish flow (when configured), emits agentgym.model.published.v1, triggers skills pipeline event, records training_completed storage events, and adds /healthz and /metrics endpoints.
Evo-Controller Integration
pmoves/services/evo-controller/agentgym_integration.py
Adds on_training_completed(...) to publish training-completed events (UTC timestamps), switches default base model to Qwen/Qwen3-8B, and uses timezone-aware timestamps for event publishing.

Sequence Diagram(s)

sequenceDiagram
    participant EC as Evo-Controller
    participant NATS as NATS
    participant RC as RL Coordinator
    participant HF as HuggingFace
    participant PL as Skills Pipeline
    participant AZ as Agent Zero

    EC->>NATS: publish agentgym.train.completed.v1 (training_run_id, trajectory_ids, model_id, fitness_metrics, timestamp)
    NATS->>RC: deliver training completion event
    RC->>RC: validate JSON payload, parse/validate UUID trajectory_ids, timestamp
    RC-->>HF: request dataset/model publish (if HF configured)
    HF-->>RC: respond with repo_url / dataset_id
    RC->>NATS: publish agentgym.model.published.v1 (training_run_id, model_id, dataset_id, repo_url, trajectory_count, source, timestamp)
    RC->>PL: emit skills.pipeline.model-benchmark-viz.v1
    RC->>AZ: optionally notify Agent Zero / observability dashboards
    RC->>Storage: record training_completed event (if storage configured)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐇 I hopped through events with whiskers bright,

training runs chimed in the midnight light.
Models boxed and datasets sent,
pipelines danced where bytes were bent.
A Qwen-bound hop — what a flight!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description covers key changes and includes a test plan, but lacks detailed testing documentation and omits the 'CHIT Contract Check' and 'Updated contracts, schemas, or topics' checkboxes required by the template. Complete the template by checking the required items (CHIT Contract Check, schema/contract updates) and providing detailed test execution results or logs.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: a new NATS event pipeline connecting EvoSwarm training completion to HuggingFace publishing via the AgentGym-RL coordinator.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/evoswarm-hf-publisher
📝 Coding Plan
  • Generate coding plan for human review comments

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

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

⚠️ Outside diff range comments (2)
pmoves/env.agentgym.example (1)

185-199: ⚠️ Potential issue | 🟡 Minor

Missing HuggingFace environment variables for the new publisher pipeline.

The PR adds HuggingFace auto-publishing functionality (in app.py lines 29-30, it reads HF_TOKEN and HF_ORG), but this example file is missing the required variables. Without these, users following this example won't be able to use the new HF publisher feature.

📝 Proposed fix: Add HuggingFace configuration section
 # A/B test traffic split (new_model:old_model)
 AGENTGYM_AB_TEST_SPLIT=0.1:0.9

+# ============================================================================
+# HuggingFace Publishing (New in PR `#935`)
+# ============================================================================
+
+# HuggingFace authentication token for auto-publishing datasets
+# Required for agentgym.train.completed.v1 → HF publishing flow
+HF_TOKEN=  # Add your HuggingFace token here
+
+# HuggingFace organization for dataset publishing (default: pmoves)
+HF_ORG=pmoves
+
 # ============================================================================
 # Storage Configuration
 # ============================================================================
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pmoves/env.agentgym.example` around lines 185 - 199, The example env file is
missing the HuggingFace variables that app.py expects (HF_TOKEN and HF_ORG are
read in app.py around the publisher logic), so add a new "HuggingFace
Publishing" section to pmoves/env.agentgym.example declaring HF_TOKEN and HF_ORG
(and optionally HF_REPO or HF_PRIVATE if your publisher supports them) with
brief comments and default/example values; ensure variable names exactly match
what app.py reads so the HF publisher code finds them at runtime.
pmoves/services/agentgym-rl-coordinator/app.py (1)

253-265: ⚠️ Potential issue | 🟡 Minor

Replace /health endpoint with /healthz and add /metrics endpoint per coding guidelines.

The service is missing the required /healthz and /metrics endpoints. The current /health endpoint does not match the standardized pattern. Reference pmoves/services/pdf-ingest/app.py for the correct implementation:

📋 Example from pdf-ingest service
`@app.get`("/healthz")
def healthz() -> Dict[str, bool]:
    return {"ok": True}

`@app.get`("/metrics")
def metrics():
    return Response(generate_latest(), media_type=CONTENT_TYPE_LATEST)

Implement /healthz to return health status (can reuse the current health_check logic) and /metrics for Prometheus-compatible metrics. As per coding guidelines: "Use FastAPI + uvicorn for API services and include the pmoves_health router for /healthz and /metrics endpoints".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pmoves/services/agentgym-rl-coordinator/app.py` around lines 253 - 265,
Replace the existing /health endpoint (function health_check) with the
standardized /healthz endpoint and add a /metrics endpoint: rename or move the
logic from the current health_check to a new healthz handler that returns the
same status fields (nats, supabase, huggingface) and register it as
`@app.get`("/healthz"); add a /metrics handler that returns Prometheus metrics
using Response(generate_latest(), media_type=CONTENT_TYPE_LATEST) (import
generate_latest and CONTENT_TYPE_LATEST from prometheus_client); alternatively
register the provided pmoves_health router which already exposes /healthz and
/metrics instead of the old /health route to comply with coding guidelines.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pmoves/env.agentgym.example`:
- Around line 14-15: Update the AGENTGYM_BASE_MODEL value to the correct
HuggingFace model identifier: replace the current placeholder string used for
the base model (AGENTGYM_BASE_MODEL) so it uses the organization prefix and
canonical name (Qwen/Qwen3-8B) instead of the incorrect "Qwen3-8B-Instruct";
locate the AGENTGYM_BASE_MODEL entry in the env.agentgym.example file and set
its value to the canonical HuggingFace ID.

In `@pmoves/services/agentgym-rl-coordinator/app.py`:
- Around line 151-158: Constructed dataset_name from training_run_id is not
validated before calling publish_to_huggingface, which can produce unclear HF
errors; before invoking hf_publisher.publish_to_huggingface, validate or
sanitize training_run_id to conform to DATASET_NAME_PATTERN (reuse the same
regex used by the /agentgym/dataset/publish endpoint), and if it fails either
(a) sanitize by replacing/stripping invalid chars to produce a valid
dataset_name or (b) log and skip publishing with a clear error; update the code
paths around training_run_id and dataset_name and ensure publish_to_huggingface
only receives a name matching DATASET_NAME_PATTERN.

In `@pmoves/services/evo-controller/agentgym_integration.py`:
- Line 385: The timestamp value is using local time while labeling it with "Z";
update the assignments that set "timestamp": time.strftime("%Y-%m-%dT%H:%M:%SZ")
to generate UTC time instead (either by passing time.gmtime() to time.strftime
or by using datetime.utcnow().strftime) so the produced ISO8601 string truly
reflects UTC; apply this change in the place where the "timestamp" key is set
and also in the publish_training_event function where the same pattern occurs.
- Around line 374-387: Refactor on_training_completed and publish_training_event
to use the standardized event publishing helpers from services.common.events
(import envelope, publish) instead of directly POSTing to agent-zero's
/events/publish; add new schema files for agentgym.train.completed.v1 and
agentgym.train.started.v1 under pmoves/contracts/schemas and register both
topics in pmoves/contracts/topics.json so envelope() can validate payloads, then
construct the event with envelope(topic, source="evo-controller", payload=...)
and call publish(envelope) to send to NATS (or the existing publish wrapper)
rather than manual HTTP calls.

---

Outside diff comments:
In `@pmoves/env.agentgym.example`:
- Around line 185-199: The example env file is missing the HuggingFace variables
that app.py expects (HF_TOKEN and HF_ORG are read in app.py around the publisher
logic), so add a new "HuggingFace Publishing" section to
pmoves/env.agentgym.example declaring HF_TOKEN and HF_ORG (and optionally
HF_REPO or HF_PRIVATE if your publisher supports them) with brief comments and
default/example values; ensure variable names exactly match what app.py reads so
the HF publisher code finds them at runtime.

In `@pmoves/services/agentgym-rl-coordinator/app.py`:
- Around line 253-265: Replace the existing /health endpoint (function
health_check) with the standardized /healthz endpoint and add a /metrics
endpoint: rename or move the logic from the current health_check to a new
healthz handler that returns the same status fields (nats, supabase,
huggingface) and register it as `@app.get`("/healthz"); add a /metrics handler
that returns Prometheus metrics using Response(generate_latest(),
media_type=CONTENT_TYPE_LATEST) (import generate_latest and CONTENT_TYPE_LATEST
from prometheus_client); alternatively register the provided pmoves_health
router which already exposes /healthz and /metrics instead of the old /health
route to comply with coding guidelines.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ffcdbf99-d390-4fb7-a5c2-c9dd7ad63a98

📥 Commits

Reviewing files that changed from the base of the PR and between b9d4eec and 5c4d3e8.

📒 Files selected for processing (4)
  • .claude/context/nats-subjects.md
  • pmoves/env.agentgym.example
  • pmoves/services/agentgym-rl-coordinator/app.py
  • pmoves/services/evo-controller/agentgym_integration.py

Comment thread pmoves/env.agentgym.example Outdated
Comment thread pmoves/services/agentgym-rl-coordinator/app.py
Comment thread pmoves/services/evo-controller/agentgym_integration.py
Comment thread pmoves/services/evo-controller/agentgym_integration.py Outdated
POWERFULMOVES added a commit that referenced this pull request Mar 15, 2026
- Rename /health → /healthz to follow PMOVES convention
- Add /metrics endpoint with Prometheus-format gauges
- Validate trajectory_ids as UUIDs before HF publish
- Fix timestamp: use datetime.now(timezone.utc) instead of local time.strftime
- Add HF_TOKEN and HF_ORG to env.agentgym.example

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@POWERFULMOVES

Copy link
Copy Markdown
Owner Author

@coderabbitai generate unit tests

@POWERFULMOVES

Copy link
Copy Markdown
Owner Author

@coderabbitai generate docstrings

@POWERFULMOVES

Copy link
Copy Markdown
Owner Author

@coderabbitai generate coding plan

@coderabbitai

coderabbitai Bot commented Mar 15, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Initiated docstring generation; will generate only if new commits exist.

@coderabbitai

coderabbitai Bot commented Mar 15, 2026

Copy link
Copy Markdown
Contributor

Note

Docstrings generation - SUCCESS
Generated docstrings for this pull request at #939

coderabbitai Bot added a commit that referenced this pull request Mar 15, 2026
Docstrings generation was requested by @POWERFULMOVES.

* #935 (comment)

The following files were modified:

* `pmoves/services/agentgym-rl-coordinator/app.py`
* `pmoves/services/evo-controller/agentgym_integration.py`
@coderabbitai

coderabbitai Bot commented Mar 15, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== agentgym-rl-coordinator/app.py ==="
fd "app.py" --full-path | grep "agentgym-rl-coordinator" | xargs cat

echo ""
echo "=== evo-controller/agentgym_integration.py ==="
fd "agentgym_integration.py" | xargs cat

echo ""
echo "=== env.agentgym.example ==="
fd "env.agentgym.example" | xargs cat

echo ""
echo "=== nats-subjects.md ==="
fd "nats-subjects.md" | xargs cat

Length of output: 57116


@POWERFULMOVES Here is a comprehensive coding plan for the EvoSwarm → AgentGym-RL → HuggingFace Publisher pipeline based on a full read of all four files.


Coding Plan: EvoSwarm → AgentGym-RL → HuggingFace Pipeline

Architecture Overview

sequenceDiagram
    participant EvoSwarm as EvoSwarm Controller
    participant AgentZero as Agent Zero (HTTP relay)
    participant NATS
    participant Coordinator as AgentGym-RL Coordinator
    participant HF as HuggingFace Hub
    participant BenchViz as Benchmark-Viz Pipeline

    EvoSwarm->>AgentZero: POST /events/publish (agentgym.train.completed.v1)
    AgentZero->>NATS: publish agentgym.train.completed.v1
    NATS->>Coordinator: training_completed_handler
    Coordinator->>HF: hf_publisher.publish_to_huggingface()
    Coordinator->>NATS: agentgym.model.published.v1
    Coordinator->>NATS: skills.pipeline.model-benchmark-viz.v1
    NATS->>BenchViz: trigger benchmark visualization
Loading

Tasks

  • 1. Fix NATS subject catalog schema & publisher/subscriber roles
  • 2. Add missing subjects (agentgym.model.published.v1, agentgym.train.started.v1) to catalog
  • 3. Add JetStream stream definition for AGENTGYM subjects
  • 4. Implement HuggingFacePublisher coordinator module (if not yet complete)
  • 5. Wire EvoSwarm to call on_training_completed after a training run resolves
  • 6. Add idempotency guard on training_completed_handler
  • 7. Add unit tests for training_completed_handler and on_training_completed
  • 8. Update env.agentgym.example with missing HF_ORGANIZATION alias note


Key Issues Found During Analysis

# File Issue Severity
1 nats-subjects.md agentgym.train.completed.v1 payload schema is completely wrong — uses old field names 🔴 High
2 nats-subjects.md Publisher/subscriber roles are reversed for agentgym.train.completed.v1 🔴 High
3 nats-subjects.md agentgym.model.published.v1 and agentgym.train.started.v1 not documented 🟡 Medium
4 app.py No idempotency guard — redelivered NATS events will double-publish to HF 🟡 Medium
5 agentgym_integration.py on_training_completed is never called from the EvoSwarm training loop 🔴 High
6 agentgym_integration.py No polling mechanism to detect when training transitions to completed 🟡 Medium
7 nats-subjects.md No JetStream stream defined for agentgym.> subjects 🟡 Medium
8 All files No unit tests for the new event-handling code 🟡 Medium

🏁 Script executed:

nats stream add AGENTGYM_EVENTS \
  --subjects "agentgym.>" \
  --storage file \
  --max-age 720h \
  --replication 1

Length of output: 42


🏁 Script executed:

#!/bin/bash
echo "=== Find coordinator package ==="
fd "coordinator" --type d
fd "coordinator" --type f | grep -v __pycache__
echo ""
echo "=== HuggingFacePublisher class ==="
ast-grep --pattern 'class HuggingFacePublisher {
  $$$
}'
echo ""
echo "=== publish_to_huggingface method ==="
ast-grep --pattern 'async def publish_to_huggingface($$$) {
  $$$
}'

Length of output: 268


🏁 Script executed:

#!/bin/bash
echo "=== Find EvoSwarm main controller ==="
fd "evo_controller" --type f
fd "swarm" --type f | grep -v __pycache__ | grep "\.py$"
echo ""
echo "=== Find where launch_agentgym_training result is consumed ==="
rg -n "launch_agentgym_training" --type py -C 5
echo ""
echo "=== Find training completion callbacks ==="
rg -n "on_training_completed" --type py -C 5

Length of output: 2346


🏁 Script executed:

#!/bin/bash
echo "=== Existing test files ==="
fd "test_" --type f | grep "\.py$"
fd "_test" --type f | grep "\.py$"
echo ""
echo "=== Test directory structure ==="
fd "tests" --type d

Length of output: 5811


🏁 Script executed:

# HuggingFace organization for published datasets
# Note: code reads HF_ORG — do not use HF_ORGANIZATION
HF_ORG=pmoves

Length of output: 0


Good findings — let me read the coordinator package and existing test files to complete the plan.


🏁 Script executed:

#!/bin/bash
echo "=== Coordinator package files ==="
find pmoves/services/agentgym-rl-coordinator/coordinator/ -type f | sort
echo ""
echo "=== Coordinator __init__.py ==="
cat pmoves/services/agentgym-rl-coordinator/coordinator/__init__.py 2>/dev/null || echo "NOT FOUND"

echo ""
echo "=== List all .py in coordinator package ==="
fd "\.py$" pmoves/services/agentgym-rl-coordinator/coordinator/

echo ""
echo "=== Evo-controller service files ==="
find pmoves/services/evo-controller/ -type f -name "*.py" | grep -v __pycache__ | sort

echo ""
echo "=== Existing agentgym tests ==="
cat pmoves/tests/services/test_agentgym_training.py
echo ""
echo "=== Evo-controller test_app.py ==="
cat pmoves/services/evo-controller/tests/test_app.py

Length of output: 7487


🏁 Script executed:

#!/bin/bash
echo "=== evo-controller/app.py ==="
cat pmoves/services/evo-controller/app.py

echo ""
echo "=== coordinator/publisher.py ==="
cat pmoves/services/agentgym-rl-coordinator/coordinator/publisher.py

echo ""
echo "=== coordinator/storage.py (first 60 lines) ==="
head -60 pmoves/services/agentgym-rl-coordinator/coordinator/storage.py

Length of output: 25549


@POWERFULMOVES Here is the complete, fully-researched coding plan based on reading all relevant files.


Coding Plan: EvoSwarm → AgentGym-RL → HuggingFace Pipeline

System Flow (as-designed)

flowchart TD
    A[EvoSwarmController._tick] -->|evaluate_training_trigger| B{Should Train?}
    B -->|No| A
    B -->|Yes| C[launch_agentgym_training\n POST /agentgym/train/start]
    C --> D[publish_training_event\n agentgym.train.started.v1]
    C --> E[poll_training_completion\n GET /agentgym/train/status/run_id]
    E -->|completed| F[on_training_completed\n POST agent-zero /events/publish]
    F --> G[NATS: agentgym.train.completed.v1]
    G --> H[training_completed_handler\n in app.py]
    H --> I[HuggingFacePublisher\n .publish_to_huggingface]
    I --> J[NATS: agentgym.model.published.v1]
    I --> K[NATS: skills.pipeline.model-benchmark-viz.v1]
    J --> L[Agent Zero / Discord Publisher]
    K --> M[Benchmark Viz Pipeline]
Loading

Tasks (7 requirements)

  • 1 🔴 Wire AgentGymIntegration into EvoSwarmController — the entire pipeline is currently dead
  • 2 🔴 Add poll_training_completion to bridge launch → completion event
  • 3 🔴 Fix README.md local-path leak in publisher.py's dataset card
  • 4 🟡 Add idempotency guard for training_completed_handler in app.py
  • 5 🔴 Fix NATS subject catalog — schema mismatch + wrong pub/sub roles + missing subjects
  • 6 🟡 Add unit tests for training_completed_handler and on_training_completed
  • 7 🟡 Add unit tests for HuggingFacePublisher


Summary of Critical Issues Found

# Severity File Problem
1 🔴 Blocker evo-controller/app.py EvoSwarmController never inherits AgentGymIntegration — entire pipeline is dead code
2 🔴 Blocker agentgym_integration.py No poll_training_completionon_training_completed is never called
3 🔴 Bug coordinator/publisher.py Dataset README embeds /tmp/agentgym_dataset_xyz local path into published HF repo
4 🔴 Docs nats-subjects.md agentgym.train.completed.v1 schema is completely wrong; pub/sub roles inverted
5 🔴 Docs nats-subjects.md agentgym.model.published.v1 and agentgym.train.started.v1 not documented
6 🟡 Risk app.py No idempotency guard — NATS redelivery causes duplicate HF publishes
7 🟡 Coverage tests/ No tests for training_completed_handler, on_training_completed, or HuggingFacePublisher

🏁 Script executed:

nats stream add AGENTGYM_EVENTS \
  --subjects "agentgym.>" \
  --storage file \
  --max-age 720h \
  --replication 1

Length of output: 42


🏁 Script executed:

#!/bin/bash
# Check if a test file for HuggingFacePublisher exists
rg -rn "HuggingFacePublisher" pmoves/tests/ pmoves/services/agentgym-rl-coordinator/ --type py 2>/dev/null | head -20
echo ""
# Check test_agentgym_storage for patterns to follow
cat pmoves/tests/services/test_agentgym_storage.py

Length of output: 2450


@POWERFULMOVES The coding plan above is complete and fully verified. One clarification on the last script run:

The rg -rn "HuggingFacePublisher" output showing n everywhere is a ripgrep flag artifact — -r n is ripgrep's replace-in-output flag consuming n as the replacement string. HuggingFacePublisher IS confirmed in all those files exactly as the cat showed. No tests for it exist yet.

The JetStream stream definition (nats stream add AGENTGYM_EVENTS ...) is a documentation-only addition to nats-subjects.md — no live nats CLI is needed for that change.


Final Plan Summary

All 7 requirements are ready to implement, in priority order:

Priority Requirement File(s)
🔴 1 Inherit AgentGymIntegration in EvoSwarmController, wire _tick() evo-controller/app.py
🔴 2 Add poll_training_completion method agentgym_integration.py
🔴 3 Fix README local path leak in dataset card coordinator/publisher.py
🔴 4 Fix NATS catalog schema + missing subjects + JetStream stream nats-subjects.md
🟡 5 Add idempotency guard for training_completed_handler agentgym-rl-coordinator/app.py
🟡 6 Unit tests for training_completed_handler + on_training_completed new test files
🟡 7 Unit tests for HuggingFacePublisher (follow test_agentgym_storage.py pattern) new test file

The test files should follow the sys.path.insert + pytest + AsyncMock + patch.object conventions established in pmoves/tests/services/test_agentgym_storage.py. Would you like me to generate the full implementation for any specific requirement?


Failed to handle agent chat message. Please try again.

@coderabbitai

coderabbitai Bot commented Mar 15, 2026

Copy link
Copy Markdown
Contributor

Request timed out after 900000ms (requestId=e663f1c2-ca1f-422d-a91e-794e711dc7f6)

@coderabbitai

coderabbitai Bot commented Mar 15, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Unit Test Generation started...

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (2)
pmoves/services/agentgym-rl-coordinator/app.py (2)

259-298: LGTM! Health and metrics endpoints properly implemented.

The /healthz endpoint matches the Docker healthcheck configuration, and /metrics provides useful Prometheus gauges for NATS, Supabase, and HuggingFace status.

Consider moving the PlainTextResponse import to the top of the file for consistency.

♻️ Move import to top of file
 from fastapi import FastAPI, HTTPException
+from fastapi.responses import PlainTextResponse
 import nats

 # ... later in metrics() ...

-    from fastapi.responses import PlainTextResponse
     return PlainTextResponse("\n".join(lines) + "\n", media_type="text/plain")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pmoves/services/agentgym-rl-coordinator/app.py` around lines 259 - 298, Move
the inline import of PlainTextResponse out of the metrics() function and add it
to the module-level imports at the top of the file; update the metrics()
function to reference PlainTextResponse directly (function name: metrics) so all
imports are centralized and consistent with other endpoints.

133-223: Consider adding idempotency guard for training completion handler.

If NATS redelivers messages (e.g., with JetStream durable subscriptions), duplicate HuggingFace publishes could occur. The PR objectives mention this as a task. A simple in-memory set or Supabase-backed check could prevent duplicate processing.

♻️ Sketch of idempotency guard
# At module level
_processed_training_runs: set[str] = set()

async def training_completed_handler(msg):
    """Handle training completion: auto-publish to HuggingFace."""
    try:
        data = json.loads(msg.data)
        training_run_id = data.get("training_run_id")
        
        # Idempotency check
        if training_run_id in _processed_training_runs:
            logger.debug("Already processed training_run_id=%s, skipping", training_run_id)
            return
        _processed_training_runs.add(training_run_id)
        
        # ... rest of handler

For persistence across restarts, check Supabase before processing.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pmoves/services/agentgym-rl-coordinator/app.py` around lines 133 - 223, Add
an idempotency guard to training_completed_handler to avoid duplicate
processing: introduce a module-level set like _processed_training_runs and check
membership of training_run_id at the start of training_completed_handler, return
early if already seen and otherwise add it before proceeding; for
durable/persistent guarantees also consult storage (e.g., call
storage.record_event or a Supabase check) to confirm the training_run_id wasn't
processed in prior runs before publishing via
hf_publisher.publish_to_huggingface, and ensure any error paths do not leave
false positives in the in-memory set (remove on failure or only add after
successful publish/record).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pmoves/services/agentgym-rl-coordinator/app.py`:
- Around line 491-500: The trajectory_id validation currently catches ValueError
but raises HTTPException without chaining; change the except block in the
trajectory_ids validation (the loop using UUID(tid)) to catch ValueError as e
and re-raise the HTTPException using "from e" so the original traceback is
preserved (i.e., in the except ValueError as e: branch raise HTTPException(... )
from e, referencing UUID, trajectory_ids, and HTTPException).
- Around line 139-162: The handler is passing trajectory_ids directly to
hf_publisher.publish_to_huggingface without UUID validation; update the block
around trajectory_ids/model_id (the same flow that logs "Training completed..."
and calls hf_publisher.publish_to_huggingface) to validate each entry in
trajectory_ids as a proper UUID (use the same validation approach as the
/agentgym/dataset/publish endpoint), and if any ID is invalid either filter them
out and log a warning or reject the publish early with a warning and return;
ensure you reference trajectory_ids and hf_publisher.publish_to_huggingface so
only validated UUIDs reach the publisher.

In `@pmoves/services/evo-controller/agentgym_integration.py`:
- Line 298: The default base_model value for AGENTGYM_BASE_MODEL is missing the
HuggingFace org prefix; update the default in agentgym_integration.py where
base_model is set (variable base_model, env var AGENTGYM_BASE_MODEL) to include
the HF namespace (e.g., "Qwen/Qwen3-8B-Instruct") so the identifier is a valid
HuggingFace model name and matches env.agentgym.example.

---

Nitpick comments:
In `@pmoves/services/agentgym-rl-coordinator/app.py`:
- Around line 259-298: Move the inline import of PlainTextResponse out of the
metrics() function and add it to the module-level imports at the top of the
file; update the metrics() function to reference PlainTextResponse directly
(function name: metrics) so all imports are centralized and consistent with
other endpoints.
- Around line 133-223: Add an idempotency guard to training_completed_handler to
avoid duplicate processing: introduce a module-level set like
_processed_training_runs and check membership of training_run_id at the start of
training_completed_handler, return early if already seen and otherwise add it
before proceeding; for durable/persistent guarantees also consult storage (e.g.,
call storage.record_event or a Supabase check) to confirm the training_run_id
wasn't processed in prior runs before publishing via
hf_publisher.publish_to_huggingface, and ensure any error paths do not leave
false positives in the in-memory set (remove on failure or only add after
successful publish/record).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 71c5b045-4fc6-43d8-a3e4-6e896f409a00

📥 Commits

Reviewing files that changed from the base of the PR and between 5c4d3e8 and 3827b4e.

📒 Files selected for processing (3)
  • pmoves/env.agentgym.example
  • pmoves/services/agentgym-rl-coordinator/app.py
  • pmoves/services/evo-controller/agentgym_integration.py

Comment thread pmoves/services/agentgym-rl-coordinator/app.py
Comment thread pmoves/services/agentgym-rl-coordinator/app.py Outdated
Comment thread pmoves/services/evo-controller/agentgym_integration.py Outdated
Comment thread pmoves/services/evo-controller/agentgym_integration.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (3)
pmoves/services/agentgym-rl-coordinator/app.py (3)

306-307: Move import to module level.

Importing PlainTextResponse inside the function adds minor overhead on each request. Move it to the top of the file with other FastAPI imports.

♻️ Proposed fix

At the top of the file (around line 10):

-from fastapi import FastAPI, HTTPException
+from fastapi import FastAPI, HTTPException
+from fastapi.responses import PlainTextResponse

In the metrics function:

-    from fastapi.responses import PlainTextResponse
     return PlainTextResponse("\n".join(lines) + "\n", media_type="text/plain")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pmoves/services/agentgym-rl-coordinator/app.py` around lines 306 - 307, The
handler currently imports PlainTextResponse inside the function (in app.py near
the metrics response), which is inefficient; move the import of
PlainTextResponse to the module-level imports alongside other FastAPI imports at
the top of app.py, then update the metrics function to return
PlainTextResponse(...) using the now-module-level symbol instead of importing it
inside the function.

133-233: Well-structured training completion handler with proper validation.

The handler correctly:

  • Validates training_run_id format against DATASET_NAME_PATTERN
  • Validates trajectory_ids as UUIDs before publishing (filtering invalid ones)
  • Uses UTC timestamps for events
  • Publishes downstream events (agentgym.model.published.v1, skills.pipeline.model-benchmark-viz.v1)
  • Logs when HF publisher is unavailable

One minor note: there's no idempotency guard to prevent duplicate HF publishes on NATS redelivery. If JetStream is used with at-least-once delivery, the same training_run_id could be published multiple times. Consider tracking published run IDs to skip duplicates.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pmoves/services/agentgym-rl-coordinator/app.py` around lines 133 - 233, Add
an idempotency guard in training_completed_handler to avoid double publishing on
NATS redelivery: before calling hf_publisher.publish_to_huggingface check a
persistent marker (e.g. storage.has_published(training_run_id) or
storage.get_event/flag) and skip publish if already marked; after a successful
publish (and after publishing agentgym.model.published.v1 and pipeline events)
record the marker (e.g. storage.record_published(training_run_id) or include it
in storage.record_event) so subsequent deliveries detect and skip the work;
apply this around hf_publisher.publish_to_huggingface and the nc.publish calls
and use the existing storage abstraction (storage.record_event or add
storage.has_published/storage.record_published) to ensure idempotency across
restarts.

53-61: HF_TOKEN is validated late at publish time, not at startup.

Per context snippet 1, HuggingFacePublisher (line 57) is initialized even if HF_TOKEN is not set. The validation happens at publish_to_huggingface() time (raises ValueError if missing). This means you won't discover a missing token until a training completion event triggers publishing.

Consider logging a warning at startup if HF_TOKEN is not configured:

💡 Optional improvement
         hf_publisher = HuggingFacePublisher(SUPABASE_URL, SUPABASE_KEY, HF_TOKEN, HF_ORG)
+        if not HF_TOKEN:
+            logger.warning("HF_TOKEN not configured - HuggingFace auto-publishing will fail")
         logger.info("Storage and coordinators initialized")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pmoves/services/agentgym-rl-coordinator/app.py` around lines 53 - 61,
HuggingFace publishing token (HF_TOKEN) is only validated later in
publish_to_huggingface(), so initialize-time will silently succeed and only fail
at publish; update the startup block that constructs HuggingFacePublisher to
validate HF_TOKEN up-front: if HF_TOKEN is present instantiate
HuggingFacePublisher(HF_TOKEN, HF_ORG, SUPABASE_URL, SUPABASE_KEY) as before,
otherwise set hf_publisher to None (or a no-op placeholder) and logger.warning
that HuggingFace publishing is disabled until HF_TOKEN is configured so missing
tokens are detected at startup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pmoves/env.agentgym.example`:
- Around line 14-19: The docker-compose default for the AGENTGYM_BASE_MODEL
environment var is inconsistent with the example; update the fallback value used
in the docker-compose agentgym service (the AGENTGYM_BASE_MODEL env entry in
docker-compose.agentgym.yml) from "Qwen2.5-7B-Instruct" to "Qwen/Qwen3-8B" so it
matches the AGENTGYM_BASE_MODEL in pmoves/env.agentgym.example; ensure only the
string value is changed and keep surrounding formatting intact.

In `@pmoves/services/evo-controller/agentgym_integration.py`:
- Line 370: The fallback base model string is inconsistent: update the default
used when computing base_model (the assignment to base_model) in
on_training_completed to match the value used in launch_agentgym_training
("Qwen/Qwen3-8B") so the same model ID is produced across flows; locate the
base_model variable assignment in on_training_completed and replace the current
"Qwen3-8B-Instruct" fallback with "Qwen/Qwen3-8B".

---

Nitpick comments:
In `@pmoves/services/agentgym-rl-coordinator/app.py`:
- Around line 306-307: The handler currently imports PlainTextResponse inside
the function (in app.py near the metrics response), which is inefficient; move
the import of PlainTextResponse to the module-level imports alongside other
FastAPI imports at the top of app.py, then update the metrics function to return
PlainTextResponse(...) using the now-module-level symbol instead of importing it
inside the function.
- Around line 133-233: Add an idempotency guard in training_completed_handler to
avoid double publishing on NATS redelivery: before calling
hf_publisher.publish_to_huggingface check a persistent marker (e.g.
storage.has_published(training_run_id) or storage.get_event/flag) and skip
publish if already marked; after a successful publish (and after publishing
agentgym.model.published.v1 and pipeline events) record the marker (e.g.
storage.record_published(training_run_id) or include it in storage.record_event)
so subsequent deliveries detect and skip the work; apply this around
hf_publisher.publish_to_huggingface and the nc.publish calls and use the
existing storage abstraction (storage.record_event or add
storage.has_published/storage.record_published) to ensure idempotency across
restarts.
- Around line 53-61: HuggingFace publishing token (HF_TOKEN) is only validated
later in publish_to_huggingface(), so initialize-time will silently succeed and
only fail at publish; update the startup block that constructs
HuggingFacePublisher to validate HF_TOKEN up-front: if HF_TOKEN is present
instantiate HuggingFacePublisher(HF_TOKEN, HF_ORG, SUPABASE_URL, SUPABASE_KEY)
as before, otherwise set hf_publisher to None (or a no-op placeholder) and
logger.warning that HuggingFace publishing is disabled until HF_TOKEN is
configured so missing tokens are detected at startup.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5406c533-2513-427f-b76f-34e4907fcaab

📥 Commits

Reviewing files that changed from the base of the PR and between 3827b4e and a3046cc.

📒 Files selected for processing (3)
  • pmoves/env.agentgym.example
  • pmoves/services/agentgym-rl-coordinator/app.py
  • pmoves/services/evo-controller/agentgym_integration.py

Comment thread pmoves/env.agentgym.example
Comment thread pmoves/services/evo-controller/agentgym_integration.py Outdated
POWERFULMOVES pushed a commit that referenced this pull request Mar 15, 2026
…w threads

New /pr-trim skill with 4-way classification (actionable / design-decision /
false-positive / nitpick), GraphQL resolveReviewThread mutation, Make targets,
NATS event schema, FlOO$ chain integration, and agent registry entry.

Tested: 36 threads resolved across PRs #935-938 in batch trim session.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
POWERFULMOVES and others added 5 commits March 15, 2026 05:29
…-publish

New NATS event chain: agentgym.train.completed.v1 → auto-publish to HF →
agentgym.model.published.v1. Updated base model default to Qwen3-8B-Instruct.
Triggers benchmark-viz pipeline on completion.

- AgentGym-RL coordinator: HF publisher with repo creation and model upload
- Evo controller: training completion callback with NATS event emission
- Updated env.agentgym.example with HF_TOKEN and HF_ORGANIZATION
- Added agentgym.* NATS subjects to context docs

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Rename /health → /healthz to follow PMOVES convention
- Add /metrics endpoint with Prometheus-format gauges
- Validate trajectory_ids as UUIDs before HF publish
- Fix timestamp: use datetime.now(timezone.utc) instead of local time.strftime
- Add HF_TOKEN and HF_ORG to env.agentgym.example

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…p to published event

The NATS training_completed_handler was bypassing DATASET_NAME_PATTERN
validation that the HTTP endpoint enforces. Also adds missing timestamp
field to agentgym.model.published.v1 for cross-service event correlation.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Addresses 4 CodeRabbit review threads:
- Thread 1+5: Fix HF model ID from 'Qwen3-8B-Instruct' to 'Qwen/Qwen3-8B'
  (env.agentgym.example + agentgym_integration.py default)
- Thread 3: Add UUID validation for trajectory_ids in NATS handler
  before HuggingFace publishing (skip invalid, log warning)
- Thread 4: Chain ValueError with 'from e' in trajectory_id validation

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Second occurrence of old 'Qwen3-8B-Instruct' default missed in previous
commit. Now all defaults use 'Qwen/Qwen3-8B'.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@POWERFULMOVES
POWERFULMOVES force-pushed the feat/evoswarm-hf-publisher branch from 41df1b7 to adef556 Compare March 15, 2026 09:30
@POWERFULMOVES
POWERFULMOVES merged commit 15994ab into main Mar 15, 2026
7 checks passed
@POWERFULMOVES
POWERFULMOVES deleted the feat/evoswarm-hf-publisher branch March 15, 2026 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants