Skip to content

Use DEFAULT_EXECUTOR constant across all entry points #98

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.

Sign up for GitHub

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

Merged
Lenar (catstrike) merged 2 commits into from
Apr 15, 2026
Merged

Use DEFAULT_EXECUTOR constant across all entry points #98

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1fe5fd0
Add install and uninstall targets to Makefile for easier tool management
catstrike Apr 15, 2026
0ba7ecf
Update executor handling and dependency version for `databao-agent`
catstrike Apr 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ setup:
uv run pytest tests/ --co -q > /dev/null
@echo "Environment ready."

install:
uv tool install --force .

uninstall:
uv tool uninstall databao

check:
uv run pre-commit run --all-files

Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ dependencies = [
"click>=8.2.1,<9.0.0",
"databao-context-engine[snowflake]~=0.7.2",
"prettytable>=3.10.0",
"databao-agent~=0.2.2",
"databao-agent~=0.2.3",
"streamlit[snowflake]>=1.53.0",
"uuid6>=2024.7.10",
"pyyaml>=6.0",
Expand Down
3 changes: 2 additions & 1 deletion src/databao_cli/features/ask/agent_factory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

from databao_cli.features.ui.project_utils import DatabaoProjectStatus, databao_project_status
from databao_cli.shared.errors import FeatureError
from databao_cli.shared.executor_utils import DEFAULT_EXECUTOR
from databao_cli.shared.project.layout import ProjectLayout


Expand Down Expand Up @@ -36,6 +37,6 @@ def initialize_agent_from_dce(project_path: Path, model: str | None, temperature
else:
llm_config = LLMConfigDirectory.DEFAULT

agent = create_agent(domain=_domain, llm_config=llm_config)
agent = create_agent(domain=_domain, llm_config=llm_config, executor_type=DEFAULT_EXECUTOR)
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

This change alters production behavior for the CLI ask flow (agent creation now explicitly pins the default executor), but the PR doesn’t add/update any tests. Please add a unit test (e.g., patching databao_cli.features.ask.agent_factory.create_agent) asserting that initialize_agent_from_dce passes the expected executor wiring for the default executor, so changing DEFAULT_EXECUTOR remains safe.

Copilot generated this review using guidance from repository custom instructions.

Comment on lines +40 to 41
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

initialize_agent_from_dce now calls create_agent(..., executor_type=DEFAULT_EXECUTOR), and DEFAULT_EXECUTOR is "separate_executor". In the Streamlit entry point, "separate_executor" is handled by passing a SeparateExecutor() instance via data_executor rather than by setting executor_type (see src/databao_cli/features/ui/app.py:131-136). If create_agent doesn’t recognize executor_type="separate_executor", this change will break the CLI ask flow. Consider mirroring the UI behavior here (special-case "separate_executor") or leave executor selection to create_agent’s default logic if that is the intended source of truth.

Suggested change
agent = create_agent(domain=_domain, llm_config=llm_config, executor_type=DEFAULT_EXECUTOR)
agent_kwargs = {
"domain": _domain,
"llm_config": llm_config,
}
if DEFAULT_EXECUTOR != "separate_executor":
agent_kwargs["executor_type"] = DEFAULT_EXECUTOR
agent = create_agent(**agent_kwargs)

Copilot uses AI. Check for mistakes.
return agent
8 changes: 6 additions & 2 deletions src/databao_cli/features/mcp/tools/databao_ask/tool.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from uuid6 import uuid6

from databao_cli.features.mcp.tools.databao_ask.agent_factory import create_agent_for_tool
from databao_cli.shared.executor_utils import DEFAULT_EXECUTOR, EXECUTOR_TYPES

if TYPE_CHECKING:
from fastmcp import FastMCP
Expand All @@ -21,6 +22,9 @@

DEFAULT_MAX_DATA_ROWS = 50

_executor_names = [f"'{k}' (default)" if k == DEFAULT_EXECUTOR else f"'{k}'" for k in EXECUTOR_TYPES]
_EXECUTOR_DESCRIPTION = f"Execution engine: {', '.join(_executor_names)}."

Comment on lines +25 to +27
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

This change makes the MCP tool schema/default executor dynamic via DEFAULT_EXECUTOR/EXECUTOR_TYPES, but the PR doesn’t add/update any tests to lock in the expected schema (default value and the description listing executor options). Please add a unit test that asserts the generated tool schema reflects DEFAULT_EXECUTOR and includes the executor options derived from EXECUTOR_TYPES so future default changes remain a one-line edit without regressions.

Copilot generated this review using guidance from repository custom instructions.

class Message(BaseModel):
"""OpenAI-compatible chat message."""
Expand Down Expand Up @@ -62,8 +66,8 @@ def databao_ask(
] = 0.0,
executor: Annotated[
str,
Field(description="Execution engine: 'claude_code' (default), 'lighthouse', 'dbt', or 'react_duckdb'."),
] = "claude_code",
Field(description=_EXECUTOR_DESCRIPTION),
] = DEFAULT_EXECUTOR,
Comment on lines 67 to +70
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

executor now defaults to DEFAULT_EXECUTOR (currently "separate_executor"), but the MCP tool’s agent factory (features/mcp/tools/databao_ask/agent_factory.py) only special-cases "claude_code" and otherwise passes executor_type=... through to create_agent. Elsewhere in the codebase, "separate_executor" is handled by instantiating and passing a SeparateExecutor instance (see src/databao_cli/features/ui/app.py:131-134). This mismatch can break the MCP tool’s default path if create_agent doesn’t accept executor_type="separate_executor". Align the MCP agent factory with the UI logic (special-case "separate_executor") or confirm/support executor_type="separate_executor" in the agent creation API.

Copilot uses AI. Check for mistakes.
max_data_rows: Annotated[
int,
Field(description="Maximum number of data rows to return in the response."),
Expand Down
Loading
Loading