Migrate CLI from manual argv parsing to tyro with component selection #42

Copilot · 2025-12-24T01:22:12Z

The issue requested replacing manual sys.argv parsing with tyro for a more structured CLI interface supporting --addr|-a flags.

Changes

Added tyro dependency (~=1.0.3) to pyproject.toml
Replaced manual argv parsing with tyro.cli() using type-annotated function
Added -a short flag as alias for --addr via tyro.conf.arg(aliases=["-a"])
Added --component parameter to selectively run DDSS components (input, output, ds, egg). Defaults to running all components when not specified.
Restructured functions: Renamed async def main() to async def run(), moved CLI argument handling function from nested to module-level def main(), and simplified cli() to call tyro.cli(main)
Preserved all existing behavior: temporary SQLite database by default, same driver mapping, identical error handling
Updated lockfile: Ran uv lock to update dependencies

Before/After

# Before: manual argv parsing
async def main(addr):
    # Run all DDSS subsystems
    
def cli():
    if len(sys.argv) == 1:
        addr = f"sqlite:///{tmpdir}/ddss.db"
    elif len(sys.argv) == 2 and sys.argv[1] not in ["--help", ...]:
        addr = sys.argv[1]
    else:
        print(f"Usage: {sys.argv[0]} [<database-addr>]")
        sys.exit(1)
    asyncio.run(main(addr))

# After: tyro with type annotations and component selection
async def run(addr, components):
    # Run selected DDSS subsystems (renamed from main)
    # Validates component names
    
def main(
    addr: Annotated[
        Optional[str],
        tyro.conf.arg(aliases=["-a"], help="Database address URL...")
    ] = None,
    component: Annotated[
        Optional[list[str]],
        tyro.conf.arg(help="Components to run...")
    ] = None,
) -> None:
    """DDSS - Distributed Deductive System Sorts
    
    Run DDSS with an interactive deductive reasoning environment.
    """
    if addr is None:
        addr = f"sqlite:///{tmpdir}/ddss.db"
    if component is None:
        component = ["ds", "egg", "input", "output"]
    asyncio.run(run(addr, component))

def cli():
    tyro.cli(main)

New Features

Component selection: Users can now specify which components to run:

ddss --component input output  # Run only input and output
ddss --component ds egg        # Run only ds and egg

Component validation: Invalid component names are rejected with clear error messages
VALID_COMPONENTS constant: Single source of truth for valid components, ensuring consistency

Tyro provides auto-generated help with formatted output and type-safe argument parsing while maintaining backward compatibility. The restructured function names better reflect their purposes: main() as the CLI entry point, run() as the async implementation, and cli() as a simple wrapper.

Original prompt

This section details on the original issue you should resolve

<issue_title>调整目前的cli</issue_title>
<issue_description>写一个使用tyro接受argv的新的main, 大概的接口是

ddss [--addr|-a ] --component|-c input output ds egg 大概这样的

-c 传递的是一个 list[str] 告诉他要启动哪些task, 具体的格式设计可以看tyro的最佳实践</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes 调整目前的cli #41

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: hzhangxyz <[email protected]>

hzhangxyz · 2025-12-24T01:34:01Z

@copilot 1. tyro 最新版本是 1.0.3 .
2. 调整 cli, main, run 三个函数的排版, main函数改名叫 run 函数, run函数改名叫main函数, 并且放在 cli 函数外面而不是局部函数

- Update tyro dependency from ~=0.9.15 to ~=1.0.3 - Rename async main() to async run() - Move nested run() function to module-level main() - Simplify cli() to just call tyro.cli(main) Co-authored-by: hzhangxyz <[email protected]>

Copilot · 2025-12-24T01:37:36Z

@copilot 1. tyro 最新版本是 1.0.3 .
2. 调整 cli, main, run 三个函数的排版, main函数改名叫 run 函数, run函数改名叫main函数, 并且放在 cli 函数外面而不是局部函��...

Done in commit 8dbe623:

Updated tyro to version 1.0.3
Restructured functions:
- async def main(addr) → async def run(addr)
- Nested def run(...) → module-level def main(...)
- cli() simplified to just call tyro.cli(main)

All tests pass (42/42) and linting is clean.

hzhangxyz · 2025-12-24T01:38:37Z

ddss/main.py

-    if len(sys.argv) == 1:
-        tmpdir = tempfile.TemporaryDirectory()
-        path = pathlib.Path(tmpdir.name) / "ddss.db"
+# Global to keep temporary directory alive during execution


不要用global 变量, 不需要考虑这个生存期

Fixed in commit 5c11325. Removed the global _tmpdir variable and its references.

hzhangxyz · 2025-12-24T01:39:35Z

ddss/main.py

            break
    else:
        print(f"Unsupported database address: {addr}")
        sys.exit(1)


用SystemExit那个

Fixed in commit 5c11325. Changed from sys.exit(1) to raise SystemExit(1).

hzhangxyz · 2025-12-24T01:40:48Z

ddss/main.py

+
+
+def cli():
+    """DDSS - Distributed Deductive System Sorts


这个函数不需要注释, 这个注释应该给main函数, 那才是 ddss --help 要看到的

Fixed in commit 5c11325. Moved the docstring from cli() to main() function. Now ddss --help displays:

DDSS - Distributed Deductive System Sorts Run DDSS with an interactive deductive reasoning environment.

hzhangxyz · 2025-12-24T01:41:00Z

pyproject.toml

    "apyds-egg~=0.0.11",
    "prompt-toolkit~=3.0.52",
    "sqlalchemy[aiosqlite,aiomysql,postgresql-asyncpg]~=2.0.45",
+    "tyro~=1.0.3",


记得重新uv lock一下

Done in commit 5c11325. Ran uv lock to update the lockfile with tyro 1.0.3 and related dependencies.

hzhangxyz · 2025-12-24T01:42:26Z

ddss/main.py

+            "If not provided, uses a temporary SQLite database."
+        )
+    ] = None,
+) -> None:


缺少一个 component: list[str] 的功能, 默认是 ["input", "output", "ds", "egg"]

Fixed in commit 5c11325. Added component: list[str] parameter with default value ["input", "output", "ds", "egg"]. Usage:

# Run all components (default) ddss # Run specific components ddss --component input output # Run only ds and egg ddss --component ds egg

…mprove docstrings - Remove global _tmpdir variable (no longer needed) - Replace sys.exit(1) with raise SystemExit(1) - Move docstring from cli() to main() for proper --help display - Add component parameter to select which components to run - Default components: ["input", "output", "ds", "egg"] - Run uv lock to update lockfile with tyro 1.0.3 Co-authored-by: hzhangxyz <[email protected]>

Copilot

Pull request overview

This PR migrates the CLI from manual sys.argv parsing to the tyro library, providing a more structured and type-safe command-line interface. The key addition is selective component execution, allowing users to run specific DDSS subsystems (ds, egg, input, output) rather than always running all of them.

Added tyro dependency (~=1.0.3) for declarative CLI argument parsing
Implemented component selection via --component parameter with validation
Added -a short flag as an alias for --addr

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.

File	Description
pyproject.toml	Added tyro dependency specification
uv.lock	Updated lock file with tyro and its dependencies (docstring-parser, typeguard)
ddss/main.py	Restructured CLI with tyro integration: renamed async main to run, created new main function with type-annotated parameters, added component selection logic with validation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ddss/main.py

Copilot · 2025-12-24T02:06:46Z

ddss/main.py


-def cli():
-    if len(sys.argv) == 1:
+VALID_COMPONENTS = ["ds", "egg", "input", "output"]


The VALID_COMPONENTS constant is defined after the run function that uses it. While Python will work at runtime since functions are executed after all module-level code runs, it would be better to define VALID_COMPONENTS before the run function for better code organization and readability.

Copilot · 2025-12-24T02:06:46Z

ddss/main.py

+    component_map = {
+        "ds": ds,
+        "egg": egg,
+        "input": input,
+        "output": output,
+    }


There is a logical issue here: the component_map is hardcoded in the run function, but the components list is passed as a parameter and validated against VALID_COMPONENTS. If component names don't exist in component_map, the code will raise a KeyError when trying to access component_map[component] on line 34. Since VALID_COMPONENTS is defined separately, there's a risk of inconsistency. Consider either defining component_map at module level along with VALID_COMPONENTS, or deriving VALID_COMPONENTS from component_map.keys() to ensure they stay in sync.

Co-authored-by: Copilot <[email protected]>

Copilot

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-24T02:16:36Z

ddss/main.py

        tmpdir = tempfile.TemporaryDirectory()
        path = pathlib.Path(tmpdir.name) / "ddss.db"
        addr = f"sqlite:///{path.as_posix()}"


The tmpdir variable is created as a TemporaryDirectory object but is never stored or cleaned up properly. This means the temporary directory will be deleted when tmpdir goes out of scope at the end of the main() function, potentially before the async run() function completes and while the database is still in use. This could cause the database file to be deleted while the application is running.

To fix this, the tmpdir should be kept alive for the entire duration of the async operation, or the cleanup should be handled explicitly in the async context.

Copilot · 2025-12-24T02:16:36Z

ddss/main.py

+        list[str],
+        tyro.conf.arg(
+            aliases=["-c"],
+            help="Components to run.",


The default value for the component parameter is missing "load" and "dump" components that are defined in component_map (lines 11-21). According to the PR description, it claims to preserve existing behavior, but the original code only ran ds, egg, input, and output. However, since load and dump are now added to the component_map, users might expect them to be available. The inconsistency is that these components exist in the map but aren't mentioned in the help text or included as a clear option.

Consider either:

Documenting which components are available (e.g., in the help text)

Validating component names against the component_map keys early with a clear error message listing valid options

Suggested change

help="Components to run.",

help="Components to run. Available: ds, egg, input, output, load, dump.",

Copilot · 2025-12-24T02:16:36Z

ddss/main.py

+        try:
+            coroutines = [component_map[component](addr, engine, session) for component in components]
+        except KeyError as e:
+            print(f"error: unsupported component {e}")


The error message format is inconsistent. When a component is invalid (line 31), the error prints error: unsupported component {e}, which will display the KeyError exception object (e.g., KeyError('invalid_component')). However, when the database is unsupported (line 83), it prints the actual address value.

For consistency and clarity, line 31 should extract and display the component name from the KeyError, such as: print(f"error: unsupported component: {component}") where component is the invalid component name.

Suggested change

print(f"error: unsupported component {e}")

component = e.args[0] if e.args else e

print(f"error: unsupported component: {component}")

Copilot · 2025-12-24T02:16:37Z

ddss/main.py

+    component: Annotated[
+        list[str],
+        tyro.conf.arg(
+            aliases=["-c"],
+            help="Components to run.",
+        ),
+    ] = ["input", "output", "ds", "egg"],
+) -> None:
+    """DDSS - Distributed Deductive System Sorts
+
+    Run DDSS with an interactive deductive environment.
+    """
+    if addr is None:
        tmpdir = tempfile.TemporaryDirectory()
        path = pathlib.Path(tmpdir.name) / "ddss.db"
        addr = f"sqlite:///{path.as_posix()}"
-    elif len(sys.argv) == 2 and sys.argv[1] not in ["--help", "-help", "-h", "/help", "/h", "/?"]:
-        addr = sys.argv[1]
-    else:
-        print(f"Usage: {sys.argv[0]} [<database-addr>]")
-        sys.exit(1)
+
    for key, value in sqlalchemy_driver.items():
        if addr.startswith(f"{key}://"):
            addr = addr.replace(f"{key}://", f"{key}+{value}://")
        if addr.startswith(f"{key}+{value}://"):
            break
    else:
-        print(f"Unsupported database address: {addr}")
-        sys.exit(1)
+        print(f"error: unsupported database: {addr}")
+        raise SystemExit(1)
+
    print(f"addr: {addr}")
-    asyncio.run(main(addr))
+    asyncio.run(run(addr, component))


The run() function signature in the diff shows parameter types addr: str, components: list[str], but when calling it at line 87, the function is invoked with component (singular) instead of components (plural). However, looking at line 24 in the actual code, the function definition uses components (plural). This appears to be correctly implemented in the actual code, but there's an inconsistency between the parameter name in main() (line 60: component) and in run() (line 24: components).

For consistency and clarity, consider using the same parameter name in both functions, preferably components (plural) since it's a list of multiple components.

Copilot · 2025-12-24T02:16:37Z

ddss/main.py



-async def main(addr):
+async def run(addr: str, components: list[str]) -> None:


The run() function is missing type annotations for its parameters. The diff shows type hints in line 24 (async def run(addr: str, components: list[str]) -> None:), but the actual implementation uses async def run(addr, components) without type annotations. Adding type annotations would improve code clarity and enable better static type checking, which is especially important since the rest of the code is being migrated to use type-annotated interfaces with tyro.

Initial plan

6905d63

Copilot AI assigned Copilot and hzhangxyz Dec 24, 2025

Copilot started work on behalf of hzhangxyz December 24, 2025 01:22 View session

Copilot AI and others added 2 commits December 24, 2025 01:28

Implement tyro-based CLI with --addr/-a flag support

7911985

Co-authored-by: hzhangxyz <[email protected]>

Address code review comments: fix resource cleanup and use sys.exit

0e6c492

Co-authored-by: hzhangxyz <[email protected]>

Copilot AI changed the title ~~[WIP] Adjust CLI to use Tyro for argument parsing~~ Migrate CLI from manual argv parsing to tyro Dec 24, 2025

Copilot finished work on behalf of hzhangxyz December 24, 2025 01:31

Copilot AI requested a review from hzhangxyz December 24, 2025 01:31

Copilot started work on behalf of hzhangxyz December 24, 2025 01:34 View session

Update tyro to 1.0.3 and restructure CLI functions

8dbe623

- Update tyro dependency from ~=0.9.15 to ~=1.0.3 - Rename async main() to async run() - Move nested run() function to module-level main() - Simplify cli() to just call tyro.cli(main) Co-authored-by: hzhangxyz <[email protected]>

Copilot finished work on behalf of hzhangxyz December 24, 2025 01:39

hzhangxyz requested changes Dec 24, 2025

View reviewed changes

Copilot started work on behalf of hzhangxyz December 24, 2025 01:43 View session

Copilot AI and others added 3 commits December 24, 2025 01:46

Update uv.lock.

ad76b56

Fix pre commit hook.

79cc843

Copilot AI changed the title ~~Migrate CLI from manual argv parsing to tyro~~ Migrate CLI from manual argv parsing to tyro with component selection Dec 24, 2025

Copilot AI requested a review from hzhangxyz December 24, 2025 01:53

Copilot finished work on behalf of hzhangxyz December 24, 2025 01:53

Update main.py

9bbefdd

hzhangxyz requested a review from Copilot December 24, 2025 01:59

Copilot started reviewing on behalf of hzhangxyz December 24, 2025 02:00 View session

hzhangxyz marked this pull request as ready for review December 24, 2025 02:03

Add two more components.

c691d30

Copilot AI reviewed Dec 24, 2025

View reviewed changes

hzhangxyz force-pushed the copilot/update-cli-with-tyro branch from 68bf1c8 to c691d30 Compare December 24, 2025 02:10

hzhangxyz requested a review from Copilot December 24, 2025 02:10

Update ddss/main.py

f45c320

Co-authored-by: Copilot <[email protected]>

Copilot started reviewing on behalf of hzhangxyz December 24, 2025 02:11 View session

hzhangxyz merged commit 67a37f6 into main Dec 24, 2025
17 checks passed

Copilot AI reviewed Dec 24, 2025

View reviewed changes

	help="Components to run.",
	help="Components to run. Available: ds, egg, input, output, load, dump.",

	print(f"error: unsupported component {e}")
	component = e.args[0] if e.args else e
	print(f"error: unsupported component: {component}")



		async def main(addr):
		async def run(addr: str, components: list[str]) -> None:

Migrate CLI from manual argv parsing to tyro with component selection #42

Migrate CLI from manual argv parsing to tyro with component selection #42

Uh oh!

Conversation

Copilot AI commented Dec 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Before/After

New Features

Comments on the Issue (you are @copilot in this section)

Uh oh!

hzhangxyz commented Dec 24, 2025

Uh oh!

Copilot AI commented Dec 24, 2025

Uh oh!

hzhangxyz Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

hzhangxyz Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

hzhangxyz Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

hzhangxyz Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

hzhangxyz Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Copilot AI commented Dec 24, 2025 •

edited

Loading