From 8c0ae3987769257c1ce2cd7d31605ea7d8a2a437 Mon Sep 17 00:00:00 2001 From: pixelcola Date: Wed, 13 May 2026 10:40:11 +0800 Subject: [PATCH 1/3] refactor: improve python code boundaries --- .trellis/spec/backend/error-handling.md | 26 +++ .../05-13-optimize-python-code/check.jsonl | 4 + .../implement.jsonl | 4 + .../tasks/05-13-optimize-python-code/prd.md | 49 ++++++ .../05-13-optimize-python-code/task.json | 26 +++ src/sing_cli/cli.py | 151 +++++++++--------- src/sing_cli/profile.py | 28 +++- 7 files changed, 204 insertions(+), 84 deletions(-) create mode 100644 .trellis/tasks/05-13-optimize-python-code/check.jsonl create mode 100644 .trellis/tasks/05-13-optimize-python-code/implement.jsonl create mode 100644 .trellis/tasks/05-13-optimize-python-code/prd.md create mode 100644 .trellis/tasks/05-13-optimize-python-code/task.json diff --git a/.trellis/spec/backend/error-handling.md b/.trellis/spec/backend/error-handling.md index 4bbee31..6306165 100644 --- a/.trellis/spec/backend/error-handling.md +++ b/.trellis/spec/backend/error-handling.md @@ -10,6 +10,32 @@ CLI 必须让失败清晰暴露。不要为了让命令“看起来成功”添 - 不使用裸 `except:`。 - 不用 `except Exception: pass`、返回 `None` 或固定成功消息吞掉失败。 +## CLI 边界实现约定 + +Typer 命令函数只捕获项目定义的预期失败类型 `SingCliError`，并在 CLI 边界统一转换为 stderr 输出和非零退出码。命令主体保持主流程代码，不在每个命令里重复写 `try/except SingCliError`。 + +#### Wrong + +```python +@app.command() +def stop() -> None: + try: + stop_service() + except SingCliError as error: + fail(error) + typer.echo("Stopped sing-box service") +``` + +#### Correct + +```python +@app.command() +@report_cli_errors +def stop() -> None: + stop_service() + typer.echo("Stopped sing-box service") +``` + ## `sing-box.exe` 解析错误 - `sing install` 默认从 `PATH` 解析 `sing-box.exe`。 diff --git a/.trellis/tasks/05-13-optimize-python-code/check.jsonl b/.trellis/tasks/05-13-optimize-python-code/check.jsonl new file mode 100644 index 0000000..efe487f --- /dev/null +++ b/.trellis/tasks/05-13-optimize-python-code/check.jsonl @@ -0,0 +1,4 @@ +{"file": ".trellis/spec/backend/index.md", "reason": "Backend command contracts and quality checklist"} +{"file": ".trellis/spec/backend/error-handling.md", "reason": "Expected CLI failure handling and explicit error boundaries"} +{"file": ".trellis/spec/backend/logging-guidelines.md", "reason": "CLI stdout and stderr output boundaries"} +{"file": ".trellis/spec/backend/quality-guidelines.md", "reason": "Python quality tools and test expectations"} diff --git a/.trellis/tasks/05-13-optimize-python-code/implement.jsonl b/.trellis/tasks/05-13-optimize-python-code/implement.jsonl new file mode 100644 index 0000000..4cfa6dd --- /dev/null +++ b/.trellis/tasks/05-13-optimize-python-code/implement.jsonl @@ -0,0 +1,4 @@ +{"file": ".trellis/spec/backend/index.md", "reason": "Backend command contracts and pre-development checklist"} +{"file": ".trellis/spec/backend/error-handling.md", "reason": "Expected CLI failure handling and explicit error boundaries"} +{"file": ".trellis/spec/backend/logging-guidelines.md", "reason": "CLI stdout and stderr output boundaries"} +{"file": ".trellis/spec/backend/quality-guidelines.md", "reason": "Python quality tools and command contracts"} diff --git a/.trellis/tasks/05-13-optimize-python-code/prd.md b/.trellis/tasks/05-13-optimize-python-code/prd.md new file mode 100644 index 0000000..ccd4d7e --- /dev/null +++ b/.trellis/tasks/05-13-optimize-python-code/prd.md @@ -0,0 +1,49 @@ +# Optimize Python code + +## Goal + +Review the project Python code and apply focused readability and maintainability improvements using `friendly-python` and `piglet` guidance, while preserving existing CLI behavior. + +## What I already know + +- The project is a Typer-based Python CLI under `src/sing_cli/`. +- The current branch is `main` and the working tree was clean before this task. +- Current backend specs require explicit CLI errors, predictable stdout/stderr, and tests for CLI behavior changes. +- `friendly-python` emphasizes clear boundaries, maintainability, and review against naming, error handling, API shape, and change isolation. +- `piglet` emphasizes single-responsibility functions, stable return types, narrow exception handling, clear naming, and explicit behavior. + +## Observed Optimization Candidates + +- `src/sing_cli/cli.py` repeats `try/except SingCliError` around most command handlers. +- `src/sing_cli/cli.py` owns timestamp parsing/formatting for list output, which may be a reusable formatting concern rather than command flow. +- Command functions mix command orchestration, state mutation, and user output; small extraction may improve readability without changing behavior. +- Existing service/profile/state modules are already small and should not be broadly rewritten without a concrete payoff. +- The user selected full project code review scope across `src/sing_cli/`. + +## Requirements + +- Preserve all existing command names, outputs, errors, state format, and tests unless a specific behavior change is explicitly approved. +- Review `src/sing_cli/cli.py`, `src/sing_cli/state.py`, `src/sing_cli/service.py`, and `src/sing_cli/profile.py`. +- Prefer focused readability improvements over broad architecture churn. +- Keep exception scopes narrow and preserve error context. +- Keep public command entry points obvious. +- Add or update tests only when behavior is intentionally touched or a refactor needs regression coverage. + +## Acceptance Criteria + +- [x] The chosen optimization scope is explicit. +- [x] Code changes are localized and improve readability or change isolation. +- [x] Ruff, ty, and pytest pass. +- [x] No backward-incompatible CLI behavior changes are introduced. + +## Out of Scope + +- Performance tuning without measured bottlenecks. +- Replacing Typer or changing the command set. +- Adding new features. +- Large rewrites across all modules. + +## Technical Notes + +- Relevant specs: `.trellis/spec/backend/index.md`, `.trellis/spec/backend/quality-guidelines.md`, `.trellis/spec/backend/error-handling.md`, `.trellis/spec/backend/logging-guidelines.md`. +- Likely files: `src/sing_cli/cli.py`, possibly small helpers in `src/sing_cli/state.py` or a narrow new module if justified. diff --git a/.trellis/tasks/05-13-optimize-python-code/task.json b/.trellis/tasks/05-13-optimize-python-code/task.json new file mode 100644 index 0000000..a318c5e --- /dev/null +++ b/.trellis/tasks/05-13-optimize-python-code/task.json @@ -0,0 +1,26 @@ +{ + "id": "optimize-python-code", + "name": "optimize-python-code", + "title": "Optimize Python code", + "description": "", + "status": "in_progress", + "dev_type": null, + "scope": null, + "package": null, + "priority": "P2", + "creator": "pixelcola", + "assignee": "pixelcola", + "createdAt": "2026-05-13", + "completedAt": null, + "branch": "refactor/optimize-python-code", + "base_branch": "main", + "worktree_path": null, + "commit": null, + "pr_url": null, + "subtasks": [], + "children": [], + "parent": null, + "relatedFiles": [], + "notes": "", + "meta": {} +} \ No newline at end of file diff --git a/src/sing_cli/cli.py b/src/sing_cli/cli.py index 68b8d60..f84e771 100644 --- a/src/sing_cli/cli.py +++ b/src/sing_cli/cli.py @@ -1,6 +1,8 @@ from __future__ import annotations +from collections.abc import Callable from datetime import datetime +from functools import wraps from pathlib import Path import typer @@ -62,6 +64,17 @@ def fail(error: SingCliError) -> None: raise typer.Exit(1) +def report_cli_errors[**P](command: Callable[P, None]) -> Callable[P, None]: + @wraps(command) + def wrapper(*args: P.args, **kwargs: P.kwargs) -> None: + try: + command(*args, **kwargs) + except SingCliError as error: + fail(error) + + return wrapper + + def to_local_timezone(value: datetime) -> datetime: return value.astimezone() @@ -79,124 +92,106 @@ def format_local_updated_at(profile_name: str, updated_at: str) -> str: @app.command() +@report_cli_errors def install(bin: Path | None = typer.Option(None, "--bin", help="Path to sing-box.exe.")) -> None: - try: - resolved_bin = resolve_bin(bin) - install_service(resolved_bin) - state = load_cli_state() - state.bin = str(resolved_bin) - save_cli_state(state) - except SingCliError as error: - fail(error) + resolved_bin = resolve_bin(bin) + install_service(resolved_bin) + state = load_cli_state() + state.bin = str(resolved_bin) + save_cli_state(state) typer.echo(f"Installed {resolved_bin}") @app.command() +@report_cli_errors def uninstall() -> None: - try: - uninstall_service() - except SingCliError as error: - fail(error) + uninstall_service() typer.echo("Uninstalled sing-box service") @app.command() +@report_cli_errors def start(name: str) -> None: - try: - state = load_cli_state() - entry = require_profile(state, name) - bin_path = require_installed_bin(state) - if service_is_running(): - raise SingCliError("sing-box service is already running. Use 'sing restart'.") - configure_service(bin_path, entry.path) - start_service() - state.active = name - save_cli_state(state) - except SingCliError as error: - fail(error) + state = load_cli_state() + entry = require_profile(state, name) + bin_path = require_installed_bin(state) + if service_is_running(): + raise SingCliError("sing-box service is already running. Use 'sing restart'.") + configure_service(bin_path, entry.path) + start_service() + state.active = name + save_cli_state(state) typer.echo(f"Started {name}") @app.command() +@report_cli_errors def stop() -> None: - try: - stop_service() - except SingCliError as error: - fail(error) + stop_service() typer.echo("Stopped sing-box service") @app.command() +@report_cli_errors def restart() -> None: - try: - state = load_cli_state() - if state.active is None: - raise SingCliError("No active profile. Run 'sing start ' first.") - entry = require_profile(state, state.active) - bin_path = require_installed_bin(state) - stop_service() - configure_service(bin_path, entry.path) - start_service() - except SingCliError as error: - fail(error) + state = load_cli_state() + if state.active is None: + raise SingCliError("No active profile. Run 'sing start ' first.") + entry = require_profile(state, state.active) + bin_path = require_installed_bin(state) + stop_service() + configure_service(bin_path, entry.path) + start_service() typer.echo(f"Restarted {state.active}") @app.command() +@report_cli_errors def add(name: str, url: str) -> None: - try: - validate_profile_name(name) - state = load_cli_state() - if name in state.profiles: - raise SingCliError(f"Profile already exists: {name}") - destination = profile_path(name) - download_profile(url, destination) - state.profiles[name] = ProfileEntry(url=url, path=str(destination), updated_at=now_utc()) - save_cli_state(state) - except SingCliError as error: - fail(error) + validate_profile_name(name) + state = load_cli_state() + if name in state.profiles: + raise SingCliError(f"Profile already exists: {name}") + destination = profile_path(name) + download_profile(url, destination) + state.profiles[name] = ProfileEntry(url=url, path=str(destination), updated_at=now_utc()) + save_cli_state(state) typer.echo(f"Added {name}") @app.command() +@report_cli_errors def remove(name: str) -> None: - try: - state = load_cli_state() - entry = require_profile(state, name) - if state.active == name: - raise SingCliError(f"Cannot remove active profile: {name}") - Path(entry.path).unlink() - del state.profiles[name] - save_cli_state(state) - except SingCliError as error: - fail(error) + state = load_cli_state() + entry = require_profile(state, name) + if state.active == name: + raise SingCliError(f"Cannot remove active profile: {name}") + Path(entry.path).unlink() + del state.profiles[name] + save_cli_state(state) typer.echo(f"Removed {name}") @app.command() +@report_cli_errors def update(name: str) -> None: - try: - state = load_cli_state() - entry = require_profile(state, name) - download_profile(entry.url, Path(entry.path)) - state.profiles[name] = ProfileEntry(url=entry.url, path=entry.path, updated_at=now_utc()) - save_cli_state(state) - except SingCliError as error: - fail(error) + state = load_cli_state() + entry = require_profile(state, name) + download_profile(entry.url, Path(entry.path)) + state.profiles[name] = ProfileEntry(url=entry.url, path=entry.path, updated_at=now_utc()) + save_cli_state(state) typer.echo(f"Updated {name}") @app.command(name="list") +@report_cli_errors def list_profiles() -> None: - try: - state = load_cli_state() + state = load_cli_state() - if not state.profiles: - typer.echo("No profiles") - return + if not state.profiles: + typer.echo("No profiles") + return - for name, entry in sorted(state.profiles.items()): - marker = "*" if state.active == name else " " - typer.echo(f"{marker} {name}\t{entry.url}\t{format_local_updated_at(name, entry.updated_at)}") - except SingCliError as error: - fail(error) + for name, entry in sorted(state.profiles.items()): + marker = "*" if state.active == name else " " + typer.echo(f"{marker} {name}\t{entry.url}\t{format_local_updated_at(name, entry.updated_at)}") diff --git a/src/sing_cli/profile.py b/src/sing_cli/profile.py index 468b817..03a5eea 100644 --- a/src/sing_cli/profile.py +++ b/src/sing_cli/profile.py @@ -16,22 +16,38 @@ def get(self, url: str) -> httpx.Response: ... def download_profile(url: str, destination: Path, client: HttpClient | None = None) -> None: if client is None: with httpx.Client(timeout=30.0, follow_redirects=True) as http_client: - _download_profile(url, destination, http_client) + download_profile_with_client(url, destination, http_client) return - _download_profile(url, destination, client) + download_profile_with_client(url, destination, client) -def _download_profile(url: str, destination: Path, http_client: HttpClient) -> None: +def download_profile_with_client(url: str, destination: Path, http_client: HttpClient) -> None: + profile_text = fetch_profile_text(url, http_client) + validate_profile_json(profile_text) + write_profile(destination, profile_text) + + +def fetch_profile_text(url: str, http_client: HttpClient) -> str: try: response = http_client.get(url) response.raise_for_status() - json.loads(response.text) - destination.parent.mkdir(parents=True, exist_ok=True) - destination.write_text(response.text, encoding="utf-8") except httpx.HTTPError as error: raise SingCliError(f"Unable to download profile: {error}") from error + + return response.text + + +def validate_profile_json(profile_text: str) -> None: + try: + json.loads(profile_text) except json.JSONDecodeError as error: raise SingCliError("Downloaded profile is not valid JSON.") from error + + +def write_profile(destination: Path, profile_text: str) -> None: + try: + destination.parent.mkdir(parents=True, exist_ok=True) + destination.write_text(profile_text, encoding="utf-8") except OSError as error: raise SingCliError(f"Unable to write profile file: {destination}") from error From cf0c965d5ba200bb10412381bd5453cefe8f9e5f Mon Sep 17 00:00:00 2001 From: pixelcola Date: Wed, 13 May 2026 10:56:39 +0800 Subject: [PATCH 2/3] chore(task): archive optimize python code --- .../2026-05}/05-13-optimize-python-code/check.jsonl | 0 .../2026-05}/05-13-optimize-python-code/implement.jsonl | 0 .../{ => archive/2026-05}/05-13-optimize-python-code/prd.md | 0 .../2026-05}/05-13-optimize-python-code/task.json | 4 ++-- 4 files changed, 2 insertions(+), 2 deletions(-) rename .trellis/tasks/{ => archive/2026-05}/05-13-optimize-python-code/check.jsonl (100%) rename .trellis/tasks/{ => archive/2026-05}/05-13-optimize-python-code/implement.jsonl (100%) rename .trellis/tasks/{ => archive/2026-05}/05-13-optimize-python-code/prd.md (100%) rename .trellis/tasks/{ => archive/2026-05}/05-13-optimize-python-code/task.json (90%) diff --git a/.trellis/tasks/05-13-optimize-python-code/check.jsonl b/.trellis/tasks/archive/2026-05/05-13-optimize-python-code/check.jsonl similarity index 100% rename from .trellis/tasks/05-13-optimize-python-code/check.jsonl rename to .trellis/tasks/archive/2026-05/05-13-optimize-python-code/check.jsonl diff --git a/.trellis/tasks/05-13-optimize-python-code/implement.jsonl b/.trellis/tasks/archive/2026-05/05-13-optimize-python-code/implement.jsonl similarity index 100% rename from .trellis/tasks/05-13-optimize-python-code/implement.jsonl rename to .trellis/tasks/archive/2026-05/05-13-optimize-python-code/implement.jsonl diff --git a/.trellis/tasks/05-13-optimize-python-code/prd.md b/.trellis/tasks/archive/2026-05/05-13-optimize-python-code/prd.md similarity index 100% rename from .trellis/tasks/05-13-optimize-python-code/prd.md rename to .trellis/tasks/archive/2026-05/05-13-optimize-python-code/prd.md diff --git a/.trellis/tasks/05-13-optimize-python-code/task.json b/.trellis/tasks/archive/2026-05/05-13-optimize-python-code/task.json similarity index 90% rename from .trellis/tasks/05-13-optimize-python-code/task.json rename to .trellis/tasks/archive/2026-05/05-13-optimize-python-code/task.json index a318c5e..60cd2bc 100644 --- a/.trellis/tasks/05-13-optimize-python-code/task.json +++ b/.trellis/tasks/archive/2026-05/05-13-optimize-python-code/task.json @@ -3,7 +3,7 @@ "name": "optimize-python-code", "title": "Optimize Python code", "description": "", - "status": "in_progress", + "status": "completed", "dev_type": null, "scope": null, "package": null, @@ -11,7 +11,7 @@ "creator": "pixelcola", "assignee": "pixelcola", "createdAt": "2026-05-13", - "completedAt": null, + "completedAt": "2026-05-13", "branch": "refactor/optimize-python-code", "base_branch": "main", "worktree_path": null, From a2c5c5e69f83952c74b9a026241a841e0f6aecef Mon Sep 17 00:00:00 2001 From: pixelcola Date: Wed, 13 May 2026 10:57:05 +0800 Subject: [PATCH 3/3] chore: record journal --- .trellis/workspace/pixelcola/index.md | 5 ++-- .trellis/workspace/pixelcola/journal-1.md | 33 +++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/.trellis/workspace/pixelcola/index.md b/.trellis/workspace/pixelcola/index.md index 64517c7..269a0e5 100644 --- a/.trellis/workspace/pixelcola/index.md +++ b/.trellis/workspace/pixelcola/index.md @@ -8,7 +8,7 @@ - **Active File**: `journal-1.md` -- **Total Sessions**: 8 +- **Total Sessions**: 9 - **Last Active**: 2026-05-13 @@ -19,7 +19,7 @@ | File | Lines | Status | |------|-------|--------| -| `journal-1.md` | ~271 | Active | +| `journal-1.md` | ~304 | Active | --- @@ -29,6 +29,7 @@ | # | Date | Title | Commits | Branch | |---|------|-------|---------|--------| +| 9 | 2026-05-13 | Optimize Python code | `8c0ae39` | `refactor/optimize-python-code` | | 8 | 2026-05-13 | Use local timezone in sing list | `c6a44bf` | `fix/list-local-timezone` | | 7 | 2026-05-12 | Expand test coverage | `764205a` | `test/improve-coverage` | | 6 | 2026-05-12 | Update README installation instructions | `49680f6` | `docs/readme-install-uv-tool` | diff --git a/.trellis/workspace/pixelcola/journal-1.md b/.trellis/workspace/pixelcola/journal-1.md index c517420..0fc94bc 100644 --- a/.trellis/workspace/pixelcola/journal-1.md +++ b/.trellis/workspace/pixelcola/journal-1.md @@ -269,3 +269,36 @@ Changed sing list to render stored UTC profile update timestamps in the computer ### Next Steps - None - task complete + + +## Session 9: Optimize Python code + +**Date**: 2026-05-13 +**Task**: Optimize Python code +**Branch**: `refactor/optimize-python-code` + +### Summary + +Refactored CLI command error handling through a shared SingCliError reporting boundary, split profile download into fetch/validate/write steps, documented the CLI boundary convention, verified pytest, Ruff, and ty, committed the refactor, and opened PR #10. + +### Main Changes + +(Add details) + +### Git Commits + +| Hash | Message | +|------|---------| +| `8c0ae39` | (see git log) | + +### Testing + +- [OK] (Add test results) + +### Status + +[OK] **Completed** + +### Next Steps + +- None - task complete