Upgrade SQL Agent Example to v0.2 - #164
Conversation
…ade-sql-agent-example
…ade-sql-agent-example
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades the SQL Agent example to v0.2 of the Agent Lightning framework and includes several bug fixes. The changes modernize the SQL agent implementation, improve error handling, and add comprehensive testing.
Key Changes
- Modernized SQL Agent to use v0.2 Agent Lightning API with new
rollout()method signature - Added comprehensive regression test for queue handling to prevent sample skipping
- Improved async execution handling in Spider evaluation utilities
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/algorithm/test_baseline.py | Added span_verbosity parameter and comprehensive queue handling regression test |
| pyproject.toml | Added gdown dependency for experiment functionality |
| examples/spider/train_sql_agent.py | New training script with multiple model configurations (fast/qwen/llama) |
| examples/spider/sql_agent.py | Modernized to v0.2 API with new rollout method signature and improved tracing |
| examples/spider/spider_eval/exec_eval.py | Updated to use new async utility function |
| examples/spider/spider_eval/async_utils.py | New utility for handling async execution in sync contexts |
| examples/spider/spider_eval/init.py | Added copyright header |
| agentlightning/tracer/base.py | Added get_langchain_handler method to base tracer |
| agentlightning/tracer/agentops.py | Improved deadlock handling and timeout configuration |
| agentlightning/store/client_server.py | Enhanced HTTP client timeout configuration and error logging |
| agentlightning/runner/agent.py | Added tracer property accessor |
| agentlightning/litagent/litagent.py | Updated tracer access to use runner when available |
| agentlightning/config.py | Added deprecation comment for lightning_cli |
| agentlightning/algorithm/fast.py | Enhanced with configurable span verbosity and improved queue handling |
| .github/workflows/examples.yml | Updated CI workflow to use new training script |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| debug=False, | ||
| db_schema=schema, | ||
| endpoint=llm.endpoint, | ||
| endpoint=llm.get_base_url(rollout.rollout_id, rollout.attempt.attempt_id), # type: ignore |
There was a problem hiding this comment.
The type: ignore comment suggests uncertainty about the API. Consider adding proper type annotations or validation to ensure the method exists and returns the expected type.
| result = agent.invoke( # type: ignore | ||
| {"question": question}, # type: ignore | ||
| {"callbacks": [self.tracer.get_langchain_callback_handler()], "recursion_limit": 100}, # type: ignore | ||
| {"callbacks": [handler] if handler else [], "recursion_limit": 100}, |
There was a problem hiding this comment.
The magic number 100 for recursion_limit should be defined as a named constant to improve maintainability and make it configurable.
| parser.add_argument( | ||
| "config", | ||
| choices=["fast", "qwen", "llama"], | ||
| help="Training configuration: 'fast' (CI testing), 'qwen' (Qwen-2.5-Coder-1.5B), 'llama' (LLaMA-3.2-3B)", |
There was a problem hiding this comment.
The help text mentions 'LLaMA-3.2-3B' but the actual model used in config_train_llama() is 'meta-llama/Llama-3.2-1B-Instruct'. The help text should be corrected to match the implementation.
There was a problem hiding this comment.
Let's fix that in another PR.
| with suppress_instrumentation(): | ||
| self._await_in_loop( | ||
| self._store.add_otel_span(self._rollout_id, self._attempt_id, span), | ||
| timeout=60.0, |
There was a problem hiding this comment.
The timeout value of 60.0 seconds should be defined as a named constant or made configurable rather than being a magic number.
This PR also includes several bug fixes.