Tinker Integration#245
Conversation
| An available port number. | ||
| """ | ||
| with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: | ||
| s.bind(("", 0)) |
Check warning
Code scanning / CodeQL
Binding a socket to all network interfaces Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 months ago
To fix the issue, the code should avoid binding the socket to all interfaces ("" or "0.0.0.0"). In the context of port discovery, it is safer and functionally equivalent to bind the socket to the loopback address (127.0.0.1), which restricts the socket to local traffic only and avoids any potential security risk. In file examples/tinker/hello.py, replace s.bind(("", 0)) with s.bind(("127.0.0.1", 0)) in the _find_available_port() function. No other changes, methods, or imports are needed.
| @@ -58,7 +58,7 @@ | ||
| An available port number. | ||
| """ | ||
| with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: | ||
| s.bind(("", 0)) | ||
| s.bind(("127.0.0.1", 0)) | ||
| return s.getsockname()[1] | ||
|
|
||
|
|
| An available port number. | ||
| """ | ||
| with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: | ||
| s.bind(("", 0)) |
Check warning
Code scanning / CodeQL
Binding a socket to all network interfaces Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 months ago
To fix this problem, change the s.bind(("", 0)) line in _find_available_port() so it binds to the loopback interface only, by specifying '127.0.0.1' instead of the empty string. This restricts the socket to accept connections only from the local machine during the port discovery. No additional imports or significant code changes are needed; just one argument needs updating in the identified function in examples/tinker/q20_train.py at line 61.
| @@ -58,7 +58,7 @@ | ||
| An available port number. | ||
| """ | ||
| with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: | ||
| s.bind(("", 0)) | ||
| s.bind(("127.0.0.1", 0)) | ||
| return s.getsockname()[1] | ||
|
|
||
|
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds a comprehensive Tinker integration example to Agent-lightning, enabling reinforcement learning fine-tuning using Tinker as the backend training service. The integration bridges Agent-lightning's rollout architecture with Tinker's training infrastructure.
Key changes:
- Added new Tinker integration example with bridge code (
agl_tinker/) to connect Agent-lightning with Tinker's RL training - Implemented two example agents: a minimal "Hello" agent and a complex 20 Questions game using CrewAI
- Added
wandbdependency to the tinker extra in project configuration
Reviewed Changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
uv.lock |
Added wandb dependency for tinker extra |
pyproject.toml |
Added wandb to tinker dependencies |
examples/tinker/README.md |
Comprehensive documentation for Tinker integration |
examples/tinker/hello.py |
Minimal training example demonstrating identity repetition |
examples/tinker/q20_*.py |
20 Questions game implementation with training and evaluation scripts |
examples/tinker/agl_tinker/*.py |
Bridge package connecting Agent-lightning with Tinker |
examples/tinker/.env.example |
Environment variable template |
examples/README.md |
Updated examples catalog with Tinker entry |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| rew = 1.0 | ||
| elif ("not " + task) in content_lower: | ||
| rew = -1.0 | ||
| elif ("you're" + task) in content_lower or ("you are" + task) in content_lower: |
There was a problem hiding this comment.
Missing space in string concatenation for 'you're' check. Should be (\"you're \" + task) and (\"you are \" + task) to properly match phrases like "you're 42" instead of "you're42".
| elif ("you're" + task) in content_lower or ("you are" + task) in content_lower: | |
| elif ("you're " + task) in content_lower or ("you are " + task) in content_lower: |
| len(val_indices), | ||
| ) | ||
| train_dataset = [train_dataset[i] for i in train_indices] | ||
| val_dataset = [train_dataset[i] for i in val_indices] |
There was a problem hiding this comment.
The validation dataset is being created from the wrong source. It should use the original train_dataset variable from line 233, but after line 251, train_dataset has been reassigned to a list. This line should reference the original dataset before reassignment. Store the original dataset in a separate variable to avoid this issue.
This pull request adds a new example integration for using Tinker as a backend training service with Agent-lightning. It introduces a comprehensive bridge package (
agl_tinker) that adapts Agent-lightning rollouts and datasets to Tinker’s reinforcement learning workflow, allowing seamless fine-tuning and evaluation workflows. The changes include documentation, environment setup, and core adapter modules that enable interoperability between the two frameworks.New Example Integration and Documentation:
tinkerexample to the catalog, with a clear note on its unmaintained status and compatibility with Agent-lightning v0.2.1.README.mdinexamples/tinker/explaining the integration, setup instructions, workflow differences, troubleshooting, and included files..env.exampletemplate for environment variables required to run the Tinker integration, including keys for Tinker, OpenAI, WANDB, and CrewAI telemetry settings.Core Bridge Package (
agl_tinker/) Implementation:agl_tinker/algo.py, implementing anAlgorithmwrapper that plugs Tinker’s training loop into Agent-lightning’s resource management and dataset system.agl_tinker/env.py, which provides adapters for Tinker’s RL environment and dataset builders, allowing Agent-lightning tasks to be used in Tinker workflows without modifying rollout logic.