-
-
Notifications
You must be signed in to change notification settings - Fork 36
Add CodeTransformAgent #1621
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add CodeTransformAgent #1621
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,182 @@ | ||||||||||||||||||||||||||||||||||||
| from __future__ import annotations | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| from typing import Any | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| import param | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| from pydantic import BaseModel, Field | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| from ...pipeline import Pipeline | ||||||||||||||||||||||||||||||||||||
| from ...sources.duckdb import DuckDBSource | ||||||||||||||||||||||||||||||||||||
| from ...util import normalize_table_name | ||||||||||||||||||||||||||||||||||||
| from ..code_executor import CodeSafetyCheck, PandasExecutor | ||||||||||||||||||||||||||||||||||||
| from ..config import PROMPTS_DIR, UserCancelledError | ||||||||||||||||||||||||||||||||||||
| from ..context import ContextModel, TContext | ||||||||||||||||||||||||||||||||||||
| from ..llm import Message | ||||||||||||||||||||||||||||||||||||
| from ..utils import describe_data, get_data | ||||||||||||||||||||||||||||||||||||
| from ..views import LumenOutput | ||||||||||||||||||||||||||||||||||||
| from .base_code import BaseCodeAgent | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| class TransformSpec(BaseModel): | ||||||||||||||||||||||||||||||||||||
| """LLM response model for pandas transformation code.""" | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| chain_of_thought: str = Field( | ||||||||||||||||||||||||||||||||||||
| description="Brief reasoning (1-2 sentences) for the transformation strategy." | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
| table_slug: str = Field( | ||||||||||||||||||||||||||||||||||||
| description=( | ||||||||||||||||||||||||||||||||||||
| "Short, descriptive snake_case name for the transformed table " | ||||||||||||||||||||||||||||||||||||
| "(e.g. filtered_orders_2024)." | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
| code: str = Field( | ||||||||||||||||||||||||||||||||||||
| description=( | ||||||||||||||||||||||||||||||||||||
| "Python code that transforms the input DataFrame `df` into a new DataFrame " | ||||||||||||||||||||||||||||||||||||
| "assigned to `df_out`. Use pandas (pd) and optionally numpy (np). " | ||||||||||||||||||||||||||||||||||||
| "Do not perform any I/O." | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| class CodeTransformInputs(ContextModel): | ||||||||||||||||||||||||||||||||||||
| data: Any | ||||||||||||||||||||||||||||||||||||
| pipeline: Pipeline | ||||||||||||||||||||||||||||||||||||
| table: str | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
| table: str |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition on line 64 states "Use when the user asks to clean, reshape, or transform data that is not easily achievable in SQL". However, this condition is vague - what defines "not easily achievable in SQL"? This could lead to confusion about when to use this agent versus a SQL agent. Consider providing more specific examples or criteria, such as "Use for complex transformations requiring iterative logic, custom Python functions, or operations not supported by SQL (e.g., advanced string manipulation, custom aggregations)".
| "Use when the user asks to clean, reshape, or transform data that is not easily achievable in SQL", | |
| "Use when the user asks to clean, reshape, or transform data in ways that are cumbersome or unsupported in SQL (e.g. iterative or row-wise logic, custom Python functions, advanced string manipulation, complex multi-step feature engineering, or custom aggregations)", |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error handling when getting available tables wraps all exceptions with a bare except clause and returns an empty list. This could hide legitimate errors (like network issues with a remote database) and make debugging difficult. Consider logging the exception or being more specific about which exceptions to catch (e.g., only catching expected exceptions like AttributeError if the source doesn't support get_tables).
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fallback table name uses string concatenation which could produce invalid table names if pipeline.table contains special characters. While normalize_table_name will clean this up, consider using a more explicit format that's clearer about the intent, such as f"{normalize_table_name(pipeline.table)}_transformed" to ensure the base table name is also normalized before concatenation.
| table_slug = normalize_table_name(output.table_slug or f"{pipeline.table}_transformed") | |
| table_slug = normalize_table_name( | |
| output.table_slug or f"{normalize_table_name(pipeline.table)}_transformed" | |
| ) |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new Pipeline created on line 150 uses the DuckDBSource with an in-memory URI (":memory:"). This creates an isolated pipeline that won't have access to any other tables from the original source. If users need to reference multiple tables or join the transformed data with other tables, they won't be able to do so. Consider documenting this limitation in the docstring, or providing a way to preserve access to other tables from the original source.
| source = DuckDBSource(uri=":memory:", mirrors={table_slug: transformed}, ephemeral=True) | |
| # Reuse the original DuckDBSource URI when possible so that the new pipeline | |
| # can still access other tables from the original source. Fall back to an | |
| # in-memory database if the original source is not a DuckDBSource or does | |
| # not expose a URI. | |
| source_uri = ":memory:" | |
| if isinstance(pipeline.source, DuckDBSource): | |
| try: | |
| source_uri = pipeline.source.uri # type: ignore[attr-defined] | |
| except AttributeError: | |
| source_uri = ":memory:" | |
| source = DuckDBSource(uri=source_uri, mirrors={table_slug: transformed}, ephemeral=True) |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the result from _generate_code_spec is None (line 171), the method returns an empty list and empty dict. However, the return type annotation indicates it should return CodeTransformOutputs, not an empty dict. This type inconsistency could cause issues for code that expects a properly typed output. Consider either raising an exception or returning a properly structured CodeTransformOutputs instance with None/default values.
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The output context on line 180 calls describe_data on the transformed data, but there's no error handling if describe_data fails. Given that this is a new DataFrame created by LLM-generated code, it could potentially have unexpected types or structures that might cause describe_data to fail. Consider wrapping this call in a try-except block to gracefully handle any potential errors, similar to the pattern used elsewhere in the codebase.
| out_context = { | |
| "code": result["code"], | |
| "pipeline": result["pipeline"], | |
| "source": result["source"], | |
| "table": result["table"], | |
| "data": await describe_data(result["data"]), | |
| try: | |
| described_data = await describe_data(result["data"]) | |
| except Exception: | |
| described_data = result["data"] | |
| out_context = { | |
| "code": result["code"], | |
| "pipeline": result["pipeline"], | |
| "source": result["source"], | |
| "table": result["table"], | |
| "data": described_data, |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new CodeTransformAgent lacks test coverage. The test file lumen/tests/ai/test_agents.py contains tests for other agents (ChatAgent, SQLAgent, VegaLiteAgent, AnalysisAgent), but no tests exist for CodeTransformAgent. At minimum, tests should cover: basic transformation execution, error handling when pipeline is missing, table_slug normalization, and validation that the transformed data is correctly exposed as a DuckDB view.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -374,3 +374,25 @@ def _validate_result(cls, result: Any) -> None: | |
| import pydeck as pdk | ||
| if not isinstance(result, pdk.Deck): | ||
| raise ValueError(f"'deck' must be a pydeck.Deck, got {type(result).__name__}") | ||
|
|
||
|
|
||
| class PandasExecutor(CodeExecutor): | ||
| """Safe executor for LLM-generated pandas transformations.""" | ||
|
|
||
| allowed_imports = ('pandas', 'pd', 'numpy', 'np') | ||
| allowed_import_prefixes = ('pandas', 'numpy') | ||
| output_variable = 'df_out' | ||
|
|
||
| @classmethod | ||
| def _get_injected_modules(cls) -> dict[str, Any]: | ||
| import numpy as np | ||
| import pandas as pd | ||
| return {'pd': pd, 'pandas': pd, 'np': np, 'numpy': np} | ||
|
Comment on lines
+387
to
+390
|
||
|
|
||
| @classmethod | ||
| def _validate_result(cls, result: Any) -> None: | ||
| import pandas as pd | ||
| if not isinstance(result, pd.DataFrame): | ||
| raise ValueError( | ||
| f"'df_out' must be a pandas.DataFrame, got {type(result).__name__}" | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| {% extends 'BaseViewAgent/code_safety.jinja2' %} | ||
|
|
||
| {% set library_name = 'pandas' %} | ||
|
|
||
| {% block safe_items %} | ||
| - Pandas operations on the provided `df` DataFrame | ||
| - Numpy operations for vectorized math (`np`) | ||
| - Assignment to `df_out` | ||
| {{ super() }} | ||
| {% endblock %} |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,22 @@ | ||||||
| {% extends 'Actor/main.jinja2' %} | ||||||
|
|
||||||
| {% block instructions %} | ||||||
| Generate Python pandas code to transform the provided DataFrame `df`. | ||||||
|
|
||||||
| Requirements: | ||||||
| - Use pandas (`pd`) and optionally numpy (`np`); both are available. | ||||||
| - Assign the final transformed DataFrame to `df_out`. | ||||||
|
||||||
| - Assign the final transformed DataFrame to `df_out`. | |
| - The input DataFrame is available as `df`; assign the final transformed DataFrame to `df_out`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The LLM is asked to provide a "snake_case" table_slug (line 27-31), but there's no validation that the LLM actually follows this instruction. The normalize_table_name function will fix most issues, but it also converts to lowercase and replaces special characters with underscores, potentially creating a table name that differs significantly from what the LLM intended. Consider adding validation feedback if the normalized name differs from the LLM-provided name, so the LLM can learn to provide properly formatted names.