Skip to content

Commit

Permalink
Fix /show for Cloud Engine (#177)
Browse files Browse the repository at this point in the history
# PR Description
### The following bug was observed: 
* When connected to the cloud engine for `arcade chat`, and the user
types `/show`, then the local environment tools are displayed. Instead,
the cloud engine's tools should be displayed.

### Why was this bug happening?:
* When a user entered the `/show` command, the CLI Command `show` was
being called directly. Since the function was a CLI command, the `local`
parameter was not being processed and resolved to its intended value
because the Typer CLI interface was being bypassed. So, the conditional
`if local:` would always evaluate to `True`.

### How this was fixed:
* I created a wrapper function for the `show` CLI Command. Now, when the
user types `/show`, then the wrapper function is called instead of the
`show` CLI command. This ensures that all input parameters are resolved
to their intended values.
  • Loading branch information
EricGustin authored Dec 19, 2024
1 parent f8c8d47 commit a4b58d9
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 37 deletions.
42 changes: 5 additions & 37 deletions arcade/arcade/cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,16 @@
from arcade.cli.display import (
display_arcade_chat_header,
display_eval_results,
display_tool_details,
display_tool_messages,
display_tools_table,
)
from arcade.cli.launcher import start_servers
from arcade.cli.show import show_logic
from arcade.cli.utils import (
OrderCommands,
compute_engine_base_url,
compute_login_url,
create_cli_catalog,
delete_deprecated_config_file,
get_eval_files,
get_tools_from_engine,
get_user_input,
handle_chat_interaction,
handle_tool_authorization,
Expand Down Expand Up @@ -177,38 +174,7 @@ def show(
"""
Show the available toolkits or detailed information about a specific tool.
"""
try:
if local:
catalog = create_cli_catalog(toolkit=toolkit)
tools = [t.definition for t in list(catalog)]
else:
tools = get_tools_from_engine(host, port, force_tls, force_no_tls, toolkit)

if tool:
# Display detailed information for the specified tool
tool_def = next(
(
t
for t in tools
if t.get_fully_qualified_name().name.lower() == tool.lower()
or str(t.get_fully_qualified_name()).lower() == tool.lower()
),
None,
)
if not tool_def:
console.print(f"❌ Tool '{tool}' not found.", style="bold red")
typer.Exit(code=1)
else:
display_tool_details(tool_def)
else:
# Display the list of tools as a table
display_tools_table(tools)

except Exception as e:
if debug:
raise
error_message = f"❌ Failed to list tools: {escape(str(e))}"
console.print(error_message, style="bold red")
show_logic(toolkit, tool, host, local, port, force_tls, force_no_tls, debug)


@cli.command(help="Start Arcade Chat in the terminal", rich_help_panel="Launch")
Expand Down Expand Up @@ -282,7 +248,9 @@ def chat(
# Add the input to history
readline.add_history(user_input)

if handle_user_command(user_input, history, host, port, force_tls, force_no_tls, show):
if handle_user_command(
user_input, history, host, port, force_tls, force_no_tls, show_logic
):
continue

history.append({"role": "user", "content": user_input})
Expand Down
54 changes: 54 additions & 0 deletions arcade/arcade/cli/show.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import typer
from rich.console import Console
from rich.markup import escape

from arcade.cli.display import display_tool_details, display_tools_table
from arcade.cli.utils import create_cli_catalog, get_tools_from_engine


def show_logic(
toolkit: str | None,
tool: str | None,
host: str,
local: bool,
port: int | None,
force_tls: bool,
force_no_tls: bool,
debug: bool,
) -> None:
"""Wrapper function for the `arcade show` CLI command
Handles the logic for showing tools/toolkits.
"""
console = Console()
try:
if local:
catalog = create_cli_catalog(toolkit=toolkit)
tools = [t.definition for t in list(catalog)]
else:
tools = get_tools_from_engine(host, port, force_tls, force_no_tls, toolkit)

if tool:
# Display detailed information for the specified tool
tool_def = next(
(
t
for t in tools
if t.get_fully_qualified_name().name.lower() == tool.lower()
or str(t.get_fully_qualified_name()).lower() == tool.lower()
),
None,
)
if not tool_def:
console.print(f"❌ Tool '{tool}' not found.", style="bold red")
typer.Exit(code=1)
else:
display_tool_details(tool_def)
else:
# Display the list of tools as a table
display_tools_table(tools)

except Exception as e:
if debug:
raise
error_message = f"❌ Failed to list tools: {escape(str(e))}"
console.print(error_message, style="bold red")
1 change: 1 addition & 0 deletions arcade/arcade/cli/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,7 @@ def handle_user_command(
toolkit=None,
tool=None,
host=host,
local=False,
port=port,
force_tls=force_tls,
force_no_tls=force_no_tls,
Expand Down
40 changes: 40 additions & 0 deletions arcade/tests/cli/test_show.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
from unittest.mock import patch

from arcade.cli.show import show_logic


def test_show_logic_local_false():
with patch("arcade.cli.show.get_tools_from_engine") as mock_get_tools:
mock_get_tools.return_value = []
show_logic(
toolkit=None,
tool=None,
host="localhost",
local=False,
port=None,
force_tls=False,
force_no_tls=False,
debug=False,
)

# get_tools_from_engine should be called when local=False
mock_get_tools.assert_called_once()


def test_show_logic_local_true():
with patch("arcade.cli.show.create_cli_catalog") as mock_create_catalog:
mock_create_catalog.return_value = []

show_logic(
toolkit=None,
tool=None,
host="localhost",
local=True,
port=None,
force_tls=False,
force_no_tls=False,
debug=False,
)

# create_cli_catalog should be called when local=True
mock_create_catalog.assert_called_once()

0 comments on commit a4b58d9

Please sign in to comment.