From 7a5925febbc65d57b6c2e3a9d3d9f583a8eaa04e Mon Sep 17 00:00:00 2001 From: harumiWeb Date: Sun, 15 Mar 2026 22:36:26 +0900 Subject: [PATCH 1/3] Add editing CLI for issue #99 phase 2 --- README.ja.md | 16 + README.md | 17 + ...ing-cli-as-public-operational-interface.md | 88 +++++ dev-docs/adr/README.md | 1 + dev-docs/adr/decision-map.md | 6 + dev-docs/adr/index.yaml | 18 + dev-docs/architecture/overview.md | 7 + dev-docs/specs/editing-api.md | 3 + dev-docs/specs/editing-cli.md | 85 +++++ docs/api.md | 2 + docs/cli.md | 61 +++- src/exstruct/cli/edit.py | 328 ++++++++++++++++++ src/exstruct/cli/main.py | 20 +- tasks/feature_spec.md | 75 ++++ tasks/todo.md | 25 ++ tests/cli/test_edit_cli.py | 292 ++++++++++++++++ 16 files changed, 1040 insertions(+), 4 deletions(-) create mode 100644 dev-docs/adr/ADR-0007-editing-cli-as-public-operational-interface.md create mode 100644 dev-docs/specs/editing-cli.md create mode 100644 src/exstruct/cli/edit.py create mode 100644 tests/cli/test_edit_cli.py diff --git a/README.ja.md b/README.ja.md index efe3e8e6..4ec53add 100644 --- a/README.ja.md +++ b/README.ja.md @@ -80,6 +80,22 @@ CLI の既定では列キーは従来どおり 0 始まりの数値文字列（` CLI の既定では shape/chart の `provenance` / `approximation_level` / `confidence` も出力しません。必要な場合は `--include-backend-metadata` を指定してください。 注意: MCP の `exstruct_extract` は `options.alpha_col=true` が既定で、CLI の既定（`false`）とは異なります。 +## クイックスタート Editing CLI + +```bash +exstruct patch --input book.xlsx --ops ops.json --backend openpyxl +exstruct patch --input book.xlsx --ops - --dry-run --pretty < ops.json +exstruct make --output new.xlsx --ops ops.json --backend openpyxl +exstruct ops list +exstruct ops describe create_chart --pretty +exstruct validate --input book.xlsx --pretty +``` + +- `patch` / `make` は JSON の `PatchResult` を標準出力に出します。 +- `ops list` / `ops describe` で public patch-op schema を確認できます。 +- `validate` はワークブックの読取可否（`is_readable`, `warnings`, `errors`）を返します。 +- Phase 2 では既存の抽出 CLI はそのまま維持し、`exstruct extract` や対話的な safety flag はまだ追加しません。 + ## MCPサーバー (標準入出力) ### uvx を使ったクイックスタート（推奨） diff --git a/README.md b/README.md index c5daab0c..91ba121d 100644 --- a/README.md +++ b/README.md @@ -90,6 +90,23 @@ By default, the CLI keeps legacy 0-based numeric string column keys (`"0"`, `"1" By default, serialized shape/chart output omits backend metadata (`provenance`, `approximation_level`, `confidence`) to reduce token usage. Use `--include-backend-metadata` or the corresponding Python/MCP option when you need it. Note: MCP `exstruct_extract` defaults to `options.alpha_col=true`, which differs from the CLI default (`false`). +## Quick Start Editing CLI + +```bash +exstruct patch --input book.xlsx --ops ops.json --backend openpyxl +exstruct patch --input book.xlsx --ops - --dry-run --pretty < ops.json +exstruct make --output new.xlsx --ops ops.json --backend openpyxl +exstruct ops list +exstruct ops describe create_chart --pretty +exstruct validate --input book.xlsx --pretty +``` + +- `patch` and `make` print JSON `PatchResult` to stdout. +- `ops list` / `ops describe` expose the public patch-op schema. +- `validate` reports workbook readability (`is_readable`, `warnings`, `errors`). +- Phase 2 keeps the legacy extraction CLI unchanged; it does not add + `exstruct extract` or interactive safety flags yet. + ## MCP Server (stdio) ### Quick Start with `uvx` (recommended) diff --git a/dev-docs/adr/ADR-0007-editing-cli-as-public-operational-interface.md b/dev-docs/adr/ADR-0007-editing-cli-as-public-operational-interface.md new file mode 100644 index 00000000..b704b244 --- /dev/null +++ b/dev-docs/adr/ADR-0007-editing-cli-as-public-operational-interface.md @@ -0,0 +1,88 @@ +# ADR-0007: Editing CLI as Public Operational Interface + +## Status + +`accepted` + +## Background + +Phase 1 established `exstruct.edit` as the first-class Python API for workbook +editing while preserving MCP as the host-owned integration layer. That still +left a gap for command-line and agent-oriented workflows: extraction already had +an `exstruct` CLI, but editing was still exposed mainly through MCP tools. + +Phase 2 needs to answer two policy questions that are likely to recur: + +- how editing commands should coexist with the legacy extraction CLI +- whether the public editing CLI should expose JSON-first operational flows + directly over `exstruct.edit` or continue to depend on MCP-facing entrypoints + +The change also touches a public CLI contract, so the compatibility and layering +decision must be recorded explicitly rather than left in implementation notes. + +## Decision + +- ExStruct adds a first-class editing CLI on the existing `exstruct` console + script with these Phase 2 commands: + - `patch` + - `make` + - `ops list` + - `ops describe` + - `validate` +- The legacy extraction entrypoint `exstruct INPUT.xlsx ...` remains valid and + is not replaced with `exstruct extract` in Phase 2. +- `patch`, `make`, and `ops*` are thin wrappers around the public + `exstruct.edit` contract. +- Editing commands are JSON-first: + - `patch` and `make` serialize `PatchResult` + - `ops list` / `ops describe` serialize patch-op schema metadata + - `validate` serializes workbook readability results +- Phase 2 does not introduce: + - interactive confirmation flows + - backup / allow-root / deny-glob flags + - a request-envelope JSON CLI format + - a new public Python validation API + +## Consequences + +- Users and agents gain a stable command-line surface for workbook editing + without routing through MCP. +- The existing extraction CLI keeps backward compatibility because editing + dispatch is opt-in by subcommand. +- The operational CLI now aligns with the public Python API, which reduces the + risk of CLI-only business logic drift. +- `validate` remains a CLI-only operational helper in Phase 2, so Python API + parity for validation is still deferred. +- The `exstruct` CLI now has two invocation styles (legacy extraction and edit + subcommands), which is slightly less uniform than a full subcommand redesign + but materially lowers migration risk. + +## Rationale + +- Tests: + - `tests/cli/test_edit_cli.py` + - `tests/cli/test_cli.py` + - `tests/cli/test_cli_alpha_col.py` + - `tests/edit/test_api.py` + - `tests/mcp/test_validate_input.py` +- Code: + - `src/exstruct/cli/main.py` + - `src/exstruct/cli/edit.py` + - `src/exstruct/edit/__init__.py` + - `src/exstruct/mcp/validate_input.py` +- Related specs: + - `dev-docs/specs/editing-api.md` + - `dev-docs/specs/editing-cli.md` + - `dev-docs/architecture/overview.md` + - `docs/cli.md` + - `docs/api.md` + - `README.md` + - `README.ja.md` + +## Supersedes + +- None + +## Superseded by + +- None diff --git a/dev-docs/adr/README.md b/dev-docs/adr/README.md index a3dfe852..07d34ff2 100644 --- a/dev-docs/adr/README.md +++ b/dev-docs/adr/README.md @@ -41,3 +41,4 @@ ADRs record what was decided, under which constraints, and which trade-offs were | `ADR-0004` | Patch Backend Selection Policy | `accepted` | `mcp` | | `ADR-0005` | PathPolicy Safety Boundary | `accepted` | `safety` | | `ADR-0006` | Public Edit API and Host-Owned Safety Boundary | `accepted` | `editing` | +| `ADR-0007` | Editing CLI as Public Operational Interface | `accepted` | `editing` | diff --git a/dev-docs/adr/decision-map.md b/dev-docs/adr/decision-map.md index 409243f5..a0e3b467 100644 --- a/dev-docs/adr/decision-map.md +++ b/dev-docs/adr/decision-map.md @@ -32,6 +32,7 @@ This document is a human-readable map for navigating ADRs by domain. - `ADR-0003` Output Serialization Omission Policy (`accepted`) - `ADR-0004` Patch Backend Selection Policy (`accepted`) +- `ADR-0007` Editing CLI as Public Operational Interface (`accepted`) ## mcp @@ -47,11 +48,16 @@ This document is a human-readable map for navigating ADRs by domain. ## editing - `ADR-0006` Public Edit API and Host-Owned Safety Boundary (`accepted`) +- `ADR-0007` Editing CLI as Public Operational Interface (`accepted`) ## api - `ADR-0006` Public Edit API and Host-Owned Safety Boundary (`accepted`) +## cli + +- `ADR-0007` Editing CLI as Public Operational Interface (`accepted`) + ## Supersession Relationships - There are currently no ADR supersession relationships. diff --git a/dev-docs/adr/index.yaml b/dev-docs/adr/index.yaml index 90ff25d5..15089ade 100644 --- a/dev-docs/adr/index.yaml +++ b/dev-docs/adr/index.yaml @@ -84,3 +84,21 @@ adrs: - dev-docs/specs/data-model.md - docs/api.md - docs/mcp.md + - id: ADR-0007 + title: Editing CLI as Public Operational Interface + status: accepted + path: dev-docs/adr/ADR-0007-editing-cli-as-public-operational-interface.md + primary_domain: editing + domains: + - editing + - cli + - compatibility + supersedes: [] + superseded_by: [] + related_specs: + - dev-docs/specs/editing-api.md + - dev-docs/specs/editing-cli.md + - docs/cli.md + - docs/api.md + - README.md + - README.ja.md diff --git a/dev-docs/architecture/overview.md b/dev-docs/architecture/overview.md index 5d1874fd..1954b0c0 100644 --- a/dev-docs/architecture/overview.md +++ b/dev-docs/architecture/overview.md @@ -41,6 +41,7 @@ exstruct/ specs.py types.py cli/ + edit.py main.py ``` @@ -84,6 +85,12 @@ PDF/PNG output (for RAG use cases) CLI entry point +- `main.py` keeps the legacy extraction CLI and dispatches to editing + subcommands only when the first token matches `patch` / `make` / `ops` / + `validate` +- `edit.py` contains the Phase 2 editing parser, JSON serialization helpers, + and wrappers around `exstruct.edit` + ### edit/ First-class public workbook editing API diff --git a/dev-docs/specs/editing-api.md b/dev-docs/specs/editing-api.md index 3dbfa5bb..c427d2ab 100644 --- a/dev-docs/specs/editing-api.md +++ b/dev-docs/specs/editing-api.md @@ -3,6 +3,9 @@ This document defines the Phase 1 public editing contract exposed from `exstruct.edit`. +Phase 2 adds a CLI wrapper around this contract; the CLI-specific surface is +documented separately in `dev-docs/specs/editing-cli.md`. + ## Public import path - Primary public package: `exstruct.edit` diff --git a/dev-docs/specs/editing-cli.md b/dev-docs/specs/editing-cli.md new file mode 100644 index 00000000..2ff68ae8 --- /dev/null +++ b/dev-docs/specs/editing-cli.md @@ -0,0 +1,85 @@ +# Editing CLI Specification + +This document defines the Phase 2 public editing CLI contract. + +## Command surface + +- Editing commands are exposed from the existing `exstruct` console script. +- Phase 2 commands: + - `exstruct patch` + - `exstruct make` + - `exstruct ops list` + - `exstruct ops describe` + - `exstruct validate` +- The legacy extraction entrypoint `exstruct INPUT.xlsx ...` remains valid and + is not rewritten to `exstruct extract` in Phase 2. + +## Dispatch and compatibility rules + +- `exstruct.cli.main` dispatches to the editing parser only when the first + token is one of the Phase 2 editing subcommands. +- All other invocations continue to use the extraction parser and + `process_excel` path unchanged. +- Phase 2 does not add a new console script or top-level Python export. + +## Patch and make commands + +- `patch` is the CLI wrapper over `exstruct.edit.patch_workbook`. +- `make` is the CLI wrapper over `exstruct.edit.make_workbook`. +- Shared request flags: + - `--sheet` + - `--on-conflict {overwrite,skip,rename}` + - `--backend {auto,openpyxl,com}` + - `--auto-formula` + - `--dry-run` + - `--return-inverse-ops` + - `--preflight-formula-check` + - `--pretty` +- `patch` requires: + - `--input PATH` + - `--ops FILE|-` +- `patch` optionally accepts `--output PATH`; when omitted, the existing patch + output defaulting behavior remains in effect. +- `make` requires `--output PATH`. +- `make` accepts optional `--ops FILE|-`; when omitted, `ops=[]`. + +## Ops input contract + +- `--ops` reads UTF-8 JSON from a file path or stdin marker `-`. +- The top-level JSON value must be an array. +- Each array item follows the existing public patch-op normalization rules + exposed from `exstruct.edit`, including alias normalization and JSON-string + op coercion. +- Phase 2 does not accept a request-envelope JSON document on the CLI. + +## Output contract + +- `patch` and `make` serialize `PatchResult` to stdout as JSON. +- `validate` serializes the existing input validation result shape: + - `is_readable` + - `warnings` + - `errors` +- `ops list` returns compact summaries with `op` and `description`. +- `ops describe` returns detailed patch-op schema metadata for one op. +- `--pretty` applies `indent=2` JSON formatting to all Phase 2 editing + commands. + +## Exit-code rules + +- `patch` / `make` exit `0` when the serialized `PatchResult` has + `error is None`; otherwise they exit `1`. +- `validate` exits `0` when `is_readable=true`; otherwise `1`. +- `ops list` exits `0` on success. +- `ops describe` exits `1` for unknown op names. +- JSON parse failures, request validation failures, and local I/O failures are + reported as stderr CLI errors and exit `1`; Phase 2 does not introduce a + separate generic JSON error envelope for these cases. + +## Explicit non-goals for Phase 2 + +- No `exstruct extract` subcommand +- No backup / confirmation / allow-root / deny-glob flags +- No summary-mode output +- No changes to backend selection or fallback policy +- No changes to MCP tool contracts +- No new public Python validation API diff --git a/docs/api.md b/docs/api.md index e011e454..cc41d8a5 100644 --- a/docs/api.md +++ b/docs/api.md @@ -105,6 +105,8 @@ Key points: MCP patch contract in Phase 1. - Use `list_patch_op_schemas()` / `get_patch_op_schema()` to inspect the public operation schema programmatically. +- The matching operational CLI is `exstruct patch`, `exstruct make`, + `exstruct ops`, and `exstruct validate`. ## Dependencies diff --git a/docs/cli.md b/docs/cli.md index b8718313..4582e9da 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -1,6 +1,11 @@ # CLI User Guide -This page explains how to run ExStruct from the command line, what each flag does, and common workflows. The CLI wraps `process_excel` under the hood. +This page explains how to run ExStruct from the command line, what each flag +does, and common workflows. + +- Extraction keeps the legacy `exstruct INPUT.xlsx ...` form and wraps + `process_excel`. +- Editing uses subcommands such as `exstruct patch` and wraps `exstruct.edit`. ## Basic usage @@ -14,6 +19,58 @@ exstruct INPUT.xlsx --format toon # TOON output (needs python-toon) - `INPUT.xlsx` supports `.xlsx/.xlsm/.xls`. - Exit code `0` on success, `1` on failure. +## Editing commands + +Phase 2 adds JSON-first editing commands while keeping the extraction entrypoint +unchanged. + +```bash +exstruct patch --input book.xlsx --ops ops.json --backend openpyxl +exstruct patch --input book.xlsx --ops - --dry-run --pretty < ops.json +exstruct make --output new.xlsx --ops ops.json --backend openpyxl +exstruct ops list +exstruct ops describe create_chart --pretty +exstruct validate --input book.xlsx --pretty +``` + +- `patch` serializes `PatchResult` to stdout and exits `1` only when + `PatchResult.error` is present. +- `make` serializes `PatchResult` for new workbook creation. +- `ops list` returns compact `{op, description}` summaries. +- `ops describe` returns the detailed schema for one patch op. +- `validate` returns input readability checks (`is_readable`, `warnings`, + `errors`). + +## Editing options + +### `patch` + +| Flag | Description | +| ---- | ----------- | +| `--input PATH` | Existing workbook to edit. | +| `--ops FILE|-` | JSON array of patch ops from a file or stdin. | +| `--output PATH` | Optional output workbook path. If omitted, the existing default patch output naming applies. | +| `--sheet TEXT` | Top-level sheet fallback for patch ops. | +| `--on-conflict {overwrite,skip,rename}` | Output conflict policy. | +| `--backend {auto,openpyxl,com}` | Backend selection. | +| `--auto-formula` | Treat `=...` values in `set_value` ops as formulas. | +| `--dry-run` | Simulate changes without saving. | +| `--return-inverse-ops` | Return inverse ops when supported. | +| `--preflight-formula-check` | Run formula-health validation before saving when supported. | +| `--pretty` | Pretty-print JSON output. | + +### `make` + +`make` accepts the same flags as `patch`, except that `--output PATH` is +required and `--input` is not used. `--ops` is optional; omitting it creates an +empty workbook. + +### `ops` and `validate` + +- `exstruct ops list [--pretty]` +- `exstruct ops describe OP [--pretty]` +- `exstruct validate --input PATH [--pretty]` + ## Options | Flag | Description | @@ -59,6 +116,8 @@ exstruct sample.xlsx --pdf --image --dpi 144 -o out.json ## Notes - Optional dependencies are lazy-imported. Missing packages raise a `MissingDependencyError` with install hints. +- Editing commands are JSON-first and do not add interactive confirmation, + backup creation, or path-restriction flags in Phase 2. - On non-COM environments, prefer `--mode libreoffice` for best-effort rich extraction on `.xlsx/.xlsm`, or `--mode light` for minimal extraction. - `--mode libreoffice` is best-effort, not a strict subset of COM output. It does not render PDFs/PNGs and does not compute auto page-break areas in v1. - `--mode libreoffice` combined with `--pdf`, `--image`, or `--auto-page-breaks-dir` fails early with a configuration error instead of silently ignoring the option. diff --git a/src/exstruct/cli/edit.py b/src/exstruct/cli/edit.py new file mode 100644 index 00000000..8fdf5762 --- /dev/null +++ b/src/exstruct/cli/edit.py @@ -0,0 +1,328 @@ +"""CLI subcommands for workbook editing operations.""" + +from __future__ import annotations + +import argparse +import json +from pathlib import Path +import sys +from typing import Any + +from pydantic import BaseModel, ValidationError + +from exstruct.edit import ( + MakeRequest, + PatchOp, + PatchRequest, + coerce_patch_ops, + get_patch_op_schema, + list_patch_op_schemas, + make_workbook, + patch_workbook, +) +from exstruct.mcp.validate_input import ValidateInputRequest, validate_input + +_EDIT_SUBCOMMANDS = frozenset({"patch", "make", "ops", "validate"}) +_ON_CONFLICT_CHOICES = ["overwrite", "skip", "rename"] +_BACKEND_CHOICES = ["auto", "openpyxl", "com"] + + +def is_edit_subcommand(argv: list[str]) -> bool: + """Return whether argv targets the editing CLI.""" + + return bool(argv) and argv[0] in _EDIT_SUBCOMMANDS + + +def build_edit_parser() -> argparse.ArgumentParser: + """Build the edit-subcommand CLI parser.""" + + parser = argparse.ArgumentParser(description="CLI for ExStruct workbook editing.") + subparsers = parser.add_subparsers(dest="command") + + patch_parser = subparsers.add_parser("patch", help="Edit an existing workbook.") + _add_patch_like_arguments( + patch_parser, + require_input=True, + require_output=False, + require_ops=True, + ) + patch_parser.set_defaults(handler=_run_patch_command) + + make_parser = subparsers.add_parser("make", help="Create and edit a workbook.") + _add_patch_like_arguments( + make_parser, + require_input=False, + require_output=True, + require_ops=False, + ) + make_parser.set_defaults(handler=_run_make_command) + + ops_parser = subparsers.add_parser( + "ops", help="Inspect supported patch operation schemas." + ) + ops_subparsers = ops_parser.add_subparsers(dest="ops_command") + + ops_list_parser = ops_subparsers.add_parser( + "list", help="List supported patch operations." + ) + ops_list_parser.add_argument( + "--pretty", + action="store_true", + help="Pretty-print JSON output.", + ) + ops_list_parser.set_defaults(handler=_run_ops_list_command) + + ops_describe_parser = ops_subparsers.add_parser( + "describe", help="Describe one patch operation." + ) + ops_describe_parser.add_argument("op", help="Patch operation name.") + ops_describe_parser.add_argument( + "--pretty", + action="store_true", + help="Pretty-print JSON output.", + ) + ops_describe_parser.set_defaults(handler=_run_ops_describe_command) + + validate_parser = subparsers.add_parser( + "validate", help="Validate that an input workbook is readable." + ) + validate_parser.add_argument( + "--input", + type=Path, + required=True, + help="Workbook path (.xlsx/.xlsm/.xls).", + ) + validate_parser.add_argument( + "--pretty", + action="store_true", + help="Pretty-print JSON output.", + ) + validate_parser.set_defaults(handler=_run_validate_command) + + return parser + + +def run_edit_cli(argv: list[str]) -> int: + """Run the edit-subcommand CLI.""" + + parser = build_edit_parser() + args = parser.parse_args(argv) + handler = getattr(args, "handler", None) + if handler is None: + parser.print_help() + return 1 + return int(handler(args)) + + +def _add_patch_like_arguments( + parser: argparse.ArgumentParser, + *, + require_input: bool, + require_output: bool, + require_ops: bool, +) -> None: + """Add shared request arguments for patch/make commands.""" + + if require_input: + parser.add_argument( + "--input", + type=Path, + required=True, + help="Input workbook path.", + ) + parser.add_argument( + "--output", + type=Path, + required=require_output, + help=( + "Output workbook path." + if require_output + else "Optional output workbook path." + ), + ) + parser.add_argument( + "--ops", + required=require_ops, + help="Path to a JSON array of patch ops, or '-' to read from stdin.", + ) + parser.add_argument("--sheet", help="Top-level sheet fallback for patch ops.") + parser.add_argument( + "--on-conflict", + choices=_ON_CONFLICT_CHOICES, + default="overwrite", + help="Conflict policy for output workbook paths.", + ) + parser.add_argument( + "--backend", + choices=_BACKEND_CHOICES, + default="auto", + help="Patch backend selection policy.", + ) + parser.add_argument( + "--auto-formula", + action="store_true", + help="Treat '=...' values as formulas in set_value ops.", + ) + parser.add_argument( + "--dry-run", + action="store_true", + help="Compute patch diff without saving workbook changes.", + ) + parser.add_argument( + "--return-inverse-ops", + action="store_true", + help="Return inverse ops when supported.", + ) + parser.add_argument( + "--preflight-formula-check", + action="store_true", + help="Run formula-health validation before saving when supported.", + ) + parser.add_argument( + "--pretty", + action="store_true", + help="Pretty-print JSON output.", + ) + + +def _run_patch_command(args: argparse.Namespace) -> int: + """Execute the patch subcommand.""" + + try: + ops = _load_patch_ops(args.ops) + request_kwargs: dict[str, Any] = { + "xlsx_path": args.input, + "ops": ops, + "sheet": args.sheet, + "on_conflict": args.on_conflict, + "auto_formula": args.auto_formula, + "dry_run": args.dry_run, + "return_inverse_ops": args.return_inverse_ops, + "preflight_formula_check": args.preflight_formula_check, + "backend": args.backend, + } + if args.output is not None: + request_kwargs["out_dir"] = args.output.parent + request_kwargs["out_name"] = args.output.name + request = PatchRequest(**request_kwargs) + result = patch_workbook(request) + except (OSError, ValidationError, ValueError) as exc: + _print_error(exc) + return 1 + + _print_json_payload(result, pretty=args.pretty) + return 0 if result.error is None else 1 + + +def _run_make_command(args: argparse.Namespace) -> int: + """Execute the make subcommand.""" + + try: + ops = _load_patch_ops(args.ops) + request = MakeRequest( + out_path=args.output, + ops=ops, + sheet=args.sheet, + on_conflict=args.on_conflict, + auto_formula=args.auto_formula, + dry_run=args.dry_run, + return_inverse_ops=args.return_inverse_ops, + preflight_formula_check=args.preflight_formula_check, + backend=args.backend, + ) + result = make_workbook(request) + except (OSError, ValidationError, ValueError) as exc: + _print_error(exc) + return 1 + + _print_json_payload(result, pretty=args.pretty) + return 0 if result.error is None else 1 + + +def _run_ops_list_command(args: argparse.Namespace) -> int: + """Execute the ops list subcommand.""" + + payload = { + "ops": [ + {"op": schema.op, "description": schema.description} + for schema in list_patch_op_schemas() + ] + } + _print_json_payload(payload, pretty=args.pretty) + return 0 + + +def _run_ops_describe_command(args: argparse.Namespace) -> int: + """Execute the ops describe subcommand.""" + + schema = get_patch_op_schema(args.op) + if schema is None: + _print_error(ValueError(f"Unknown patch operation: {args.op}")) + return 1 + _print_json_payload(schema, pretty=args.pretty) + return 0 + + +def _run_validate_command(args: argparse.Namespace) -> int: + """Execute the validate subcommand.""" + + try: + result = validate_input(ValidateInputRequest(xlsx_path=args.input)) + except (OSError, ValidationError, ValueError) as exc: + _print_error(exc) + return 1 + + _print_json_payload(result, pretty=args.pretty) + return 0 if result.is_readable else 1 + + +def _load_patch_ops(source: str | None) -> list[PatchOp]: + """Load patch ops from a JSON file or stdin.""" + + if source is None: + return [] + payload = _load_json_value(source) + if not isinstance(payload, list): + raise ValueError("--ops must contain a JSON array.") + normalized_ops = coerce_patch_ops(payload) + return [PatchOp(**op_payload) for op_payload in normalized_ops] + + +def _load_json_value(source: str) -> object: + """Load a JSON value from a file path or stdin marker.""" + + if source == "-": + raw = sys.stdin.read() + else: + raw = Path(source).read_text(encoding="utf-8") + try: + return json.loads(raw) + except json.JSONDecodeError as exc: + raise ValueError(f"Invalid JSON in --ops: {exc.msg}") from exc + + +def _print_json_payload(payload: object, *, pretty: bool) -> None: + """Serialize one JSON payload to stdout.""" + + serializable: object + if isinstance(payload, BaseModel): + serializable = payload.model_dump(mode="json") + else: + serializable = payload + print( + json.dumps( + serializable, + ensure_ascii=False, + indent=2 if pretty else None, + ), + flush=True, + ) + + +def _print_error(exc: Exception) -> None: + """Print one CLI error to stderr.""" + + print(f"Error: {exc}", file=sys.stderr, flush=True) + + +__all__ = ["build_edit_parser", "is_edit_subcommand", "run_edit_cli"] diff --git a/src/exstruct/cli/main.py b/src/exstruct/cli/main.py index 481606e0..c3d66d3b 100644 --- a/src/exstruct/cli/main.py +++ b/src/exstruct/cli/main.py @@ -1,4 +1,4 @@ -"""Command-line interface for running ExStruct extraction.""" +"""Command-line interface for ExStruct extraction and editing.""" from __future__ import annotations @@ -8,6 +8,7 @@ from exstruct import process_excel from exstruct.cli.availability import ComAvailability, get_com_availability +from exstruct.cli.edit import is_edit_subcommand, run_edit_cli def _ensure_utf8_stdout() -> None: @@ -56,7 +57,16 @@ def build_parser( Configured argument parser. """ parser = argparse.ArgumentParser( - description="Dev-only CLI stub for ExStruct extraction." + description="CLI for ExStruct extraction.", + epilog=( + "Editing commands:\n" + " exstruct patch --input book.xlsx --ops ops.json\n" + " exstruct make --output new.xlsx --ops ops.json\n" + " exstruct ops list\n" + " exstruct ops describe create_chart\n" + " exstruct validate --input book.xlsx" + ), + formatter_class=argparse.RawDescriptionHelpFormatter, ) parser.add_argument("input", type=Path, help="Excel file (.xlsx/.xlsm/.xls)") parser.add_argument( @@ -150,8 +160,12 @@ def main(argv: list[str] | None = None) -> int: Exit code (0 for success, 1 for failure). """ _ensure_utf8_stdout() + resolved_argv = list(sys.argv[1:] if argv is None else argv) + if is_edit_subcommand(resolved_argv): + return run_edit_cli(resolved_argv) + parser = build_parser() - args = parser.parse_args(argv) + args = parser.parse_args(resolved_argv) input_path: Path = args.input if not input_path.exists(): diff --git a/tasks/feature_spec.md b/tasks/feature_spec.md index 140f86d4..3275f5d3 100644 --- a/tasks/feature_spec.md +++ b/tasks/feature_spec.md @@ -94,3 +94,78 @@ - `dev-docs/specs/data-model.md` の “actual locations” 先頭 bullet が import path と言いながら filesystem path を示している。 - `dev-docs/architecture/overview.md` の `edit/` tree に `chart_types.py` と `errors.py` が抜けている。 + +## 2026-03-15 issue #99 phase 2 editing CLI + +### Goal + +- Excel editing を first-class CLI として公開し、`exstruct.edit` を薄く包む operational interface を追加する。 +- 既存の抽出 CLI `exstruct INPUT.xlsx ...` は互換維持し、編集系だけ subcommand を追加する。 +- `PatchResult` と既存 patch op/schema 契約を崩さず、agent 向けに JSON-first な CLI を提供する。 + +### Public contract + +- New CLI subcommands: + - `exstruct patch` + - `exstruct make` + - `exstruct ops list` + - `exstruct ops describe` + - `exstruct validate` +- Chosen Phase 2 boundaries: + - keep legacy extraction CLI entrypoint unchanged + - do not add `exstruct extract` in this phase + - `validate` means input-file readability validation, not patch request static validation + - default output for new edit commands is JSON to stdout +- `patch` contract: + - required flags: `--input`, `--ops` + - optional flags: `--output`, `--sheet`, `--on-conflict`, `--backend`, `--auto-formula`, `--dry-run`, `--return-inverse-ops`, `--preflight-formula-check`, `--pretty` + - `--ops` accepts a top-level JSON array from file or stdin (`-`) + - exit `0` when `PatchResult.error is None`; otherwise emit serialized `PatchResult` and exit `1` +- `make` contract: + - required flag: `--output` + - optional flags: `--ops`, `--sheet`, `--on-conflict`, `--backend`, `--auto-formula`, `--dry-run`, `--return-inverse-ops`, `--preflight-formula-check`, `--pretty` + - omitted `--ops` defaults to `[]` +- `ops` contract: + - `list` returns compact JSON summaries (`op`, `description`) + - `describe` returns detailed schema metadata for one op +- `validate` contract: + - required flag: `--input` + - output shape follows the existing input validation result (`is_readable`, `warnings`, `errors`) + +### Implementation boundary + +- `patch` / `make` / `ops` use `exstruct.edit` as the primary integration surface. +- `validate` may reuse the existing validation logic, but must not require MCP `PathPolicy`. +- Phase 2 does not change: + - backend selection/fallback policy + - patch result schema + - MCP tool payloads or server safety policy +- Phase 2 also excludes: + - backup / confirmation / allow-root / deny-glob flags + - summary-mode output + - request-envelope JSON input + +### ADR verdict + +- `adr-suggester`: `required` +- rationale: public CLI contract and CLI/API/MCP responsibility alignment change at policy level, while legacy extraction CLI compatibility is intentionally preserved. +- existing ADR candidates: + - `ADR-0006-public-edit-api-and-host-boundary` + - `ADR-0005-path-policy-safety-boundary` + - `ADR-0004-patch-backend-selection-policy` +- suggested next action: `new-adr` +- candidate ADR title: `Editing CLI as Public Operational Interface` + +### Permanent references + +- ADR: + - `dev-docs/adr/ADR-0007-editing-cli-as-public-operational-interface.md` +- Internal specs: + - `dev-docs/specs/editing-api.md` + - `dev-docs/specs/editing-cli.md` + - `dev-docs/architecture/overview.md` +- Public docs: + - `docs/cli.md` + - `docs/api.md` + - `README.md` + - `README.ja.md` diff --git a/tasks/todo.md b/tasks/todo.md index 1799a047..233fc9cb 100644 --- a/tasks/todo.md +++ b/tasks/todo.md @@ -82,3 +82,28 @@ - Verification: - `git diff --check` - `uv run task precommit-run` + +## 2026-03-15 issue #99 phase 2 editing CLI + +### Planning + +- [x] issue #99 の Phase 2 を CLI 追加として固定し、legacy extraction CLI との互換境界を明文化する +- [x] `patch` / `make` / `ops list` / `ops describe` / `validate` の CLI surface を実装する +- [x] `patch` / `make` を `exstruct.edit` に接続し、JSON-first 出力と exit code 契約を整える +- [x] `validate` を入力ファイル検証 CLI として追加し、MCP `PathPolicy` なしで動作させる +- [x] CLI 回帰/新規テストを追加し、legacy extraction path の互換を確認する +- [x] ADR / internal spec / public docs / README.ja parity を更新する +- [x] targeted pytest と `uv run task precommit-run` を実行する + +### Review + +- `src/exstruct/cli/edit.py` を追加し、`patch` / `make` / `ops` / `validate` の editing CLI を実装した。 +- `src/exstruct/cli/main.py` は first-token dispatch で editing subcommands だけを edit parser に回し、legacy extraction CLI はそのまま維持した。 +- `patch` / `make` は `exstruct.edit` を呼び、`PatchResult` JSON を stdout に出したうえで `error is None` のときだけ exit `0` にした。 +- `validate` は既存の入力ファイル検証ロジックを CLI に昇格し、`is_readable` / `warnings` / `errors` を JSON で返すようにした。 +- 恒久文書として `dev-docs/specs/editing-cli.md` と `dev-docs/adr/ADR-0007-editing-cli-as-public-operational-interface.md` を追加し、ADR index artifacts と CLI/API/README 文書を更新した。 +- Verification: + - `uv run pytest tests/cli/test_edit_cli.py tests/cli/test_cli.py tests/cli/test_cli_alpha_col.py tests/edit/test_api.py tests/mcp/test_validate_input.py -q` + - `uv run pytest tests/core/test_mode_output.py -q` + - `uv run task precommit-run` + - `git diff --check` diff --git a/tests/cli/test_edit_cli.py b/tests/cli/test_edit_cli.py new file mode 100644 index 00000000..fa68abf1 --- /dev/null +++ b/tests/cli/test_edit_cli.py @@ -0,0 +1,292 @@ +from __future__ import annotations + +from contextlib import redirect_stderr, redirect_stdout +import io +import json +from pathlib import Path + +from openpyxl import Workbook +from pydantic import BaseModel +import pytest + +from exstruct.cli.availability import ComAvailability +import exstruct.cli.edit as edit_cli_module +from exstruct.cli.edit import is_edit_subcommand +import exstruct.cli.main as cli_main_module +from exstruct.cli.main import build_parser, main as cli_main + + +class CliResult(BaseModel): + """Captured result from one in-process CLI run.""" + + returncode: int + stdout: str + stderr: str + + +def _create_workbook(path: Path) -> None: + workbook = Workbook() + sheet = workbook.active + assert sheet is not None + sheet.title = "Sheet1" + sheet["A1"] = "old" + workbook.save(path) + workbook.close() + + +def _run_cli(args: list[str], *, stdin_text: str | None = None) -> CliResult: + stdout_buffer = io.StringIO() + stderr_buffer = io.StringIO() + original_stdin = io.StringIO(stdin_text) if stdin_text is not None else None + with redirect_stdout(stdout_buffer), redirect_stderr(stderr_buffer): + if original_stdin is None: + returncode = cli_main(argv=args) + else: + import sys + + previous_stdin = sys.stdin + try: + sys.stdin = original_stdin + returncode = cli_main(argv=args) + finally: + sys.stdin = previous_stdin + return CliResult( + returncode=returncode, + stdout=stdout_buffer.getvalue(), + stderr=stderr_buffer.getvalue(), + ) + + +def test_patch_cli_returns_patch_result_json(tmp_path: Path) -> None: + source = tmp_path / "book.xlsx" + ops_path = tmp_path / "ops.json" + _create_workbook(source) + ops_path.write_text( + json.dumps( + [{"op": "set_value", "sheet": "Sheet1", "cell": "A1", "value": "new"}] + ), + encoding="utf-8", + ) + + result = _run_cli( + [ + "patch", + "--input", + str(source), + "--ops", + str(ops_path), + "--backend", + "openpyxl", + ] + ) + + assert result.returncode == 0 + payload = json.loads(result.stdout) + assert payload["engine"] == "openpyxl" + assert payload["error"] is None + assert Path(payload["out_path"]).exists() + assert result.stderr == "" + + +def test_patch_cli_reads_ops_from_stdin(tmp_path: Path) -> None: + source = tmp_path / "book.xlsx" + _create_workbook(source) + + result = _run_cli( + [ + "patch", + "--input", + str(source), + "--ops", + "-", + "--backend", + "openpyxl", + ], + stdin_text=json.dumps( + [{"op": "set_value", "sheet": "Sheet1", "cell": "A1", "value": "stdin"}] + ), + ) + + assert result.returncode == 0 + payload = json.loads(result.stdout) + assert payload["error"] is None + assert Path(payload["out_path"]).exists() + + +def test_patch_cli_returns_nonzero_for_invalid_ops_json(tmp_path: Path) -> None: + source = tmp_path / "book.xlsx" + ops_path = tmp_path / "ops.json" + _create_workbook(source) + ops_path.write_text("{bad json", encoding="utf-8") + + result = _run_cli(["patch", "--input", str(source), "--ops", str(ops_path)]) + + assert result.returncode == 1 + assert "Invalid JSON in --ops" in result.stderr + + +def test_patch_cli_returns_nonzero_when_patch_result_contains_error( + tmp_path: Path, +) -> None: + source = tmp_path / "book.xlsx" + ops_path = tmp_path / "ops.json" + _create_workbook(source) + ops_path.write_text( + json.dumps( + [{"op": "set_value", "sheet": "Missing", "cell": "A1", "value": "new"}] + ), + encoding="utf-8", + ) + + result = _run_cli( + [ + "patch", + "--input", + str(source), + "--ops", + str(ops_path), + "--backend", + "openpyxl", + ] + ) + + assert result.returncode == 1 + payload = json.loads(result.stdout) + assert payload["error"] is not None + assert payload["error"]["sheet"] == "Missing" + + +def test_make_cli_creates_workbook_and_returns_json(tmp_path: Path) -> None: + output = tmp_path / "created.xlsx" + ops_path = tmp_path / "ops.json" + ops_path.write_text( + json.dumps( + [ + {"op": "add_sheet", "sheet": "Data"}, + {"op": "set_value", "sheet": "Data", "cell": "A1", "value": "ok"}, + ] + ), + encoding="utf-8", + ) + + result = _run_cli( + [ + "make", + "--output", + str(output), + "--ops", + str(ops_path), + "--backend", + "openpyxl", + ] + ) + + assert result.returncode == 0 + payload = json.loads(result.stdout) + assert payload["error"] is None + assert output.exists() + + +def test_make_cli_defaults_to_empty_ops(tmp_path: Path) -> None: + output = tmp_path / "empty.xlsx" + + result = _run_cli(["make", "--output", str(output), "--backend", "openpyxl"]) + + assert result.returncode == 0 + payload = json.loads(result.stdout) + assert payload["error"] is None + assert output.exists() + + +def test_ops_list_cli_returns_compact_schema_summary() -> None: + result = _run_cli(["ops", "list"]) + + assert result.returncode == 0 + payload = json.loads(result.stdout) + assert "ops" in payload + assert any(item["op"] == "set_value" for item in payload["ops"]) + + +def test_ops_describe_cli_returns_schema_detail() -> None: + result = _run_cli(["ops", "describe", "create_chart"]) + + assert result.returncode == 0 + payload = json.loads(result.stdout) + assert payload["op"] == "create_chart" + assert "required" in payload + assert "example" in payload + + +def test_ops_describe_cli_rejects_unknown_op() -> None: + result = _run_cli(["ops", "describe", "missing_op"]) + + assert result.returncode == 1 + assert "Unknown patch operation" in result.stderr + + +def test_validate_cli_returns_json_for_readable_file(tmp_path: Path) -> None: + path = tmp_path / "input.xlsx" + path.write_bytes(b"x") + + result = _run_cli(["validate", "--input", str(path)]) + + assert result.returncode == 0 + payload = json.loads(result.stdout) + assert payload["is_readable"] is True + + +def test_validate_cli_returns_nonzero_for_missing_file(tmp_path: Path) -> None: + path = tmp_path / "missing.xlsx" + + result = _run_cli(["validate", "--input", str(path)]) + + assert result.returncode == 1 + payload = json.loads(result.stdout) + assert payload["is_readable"] is False + assert payload["errors"] + + +def test_validate_cli_returns_nonzero_when_validation_raises( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + path = tmp_path / "input.xlsx" + path.write_bytes(b"x") + + def _raise_os_error(_request: object) -> object: + raise OSError("boom") + + monkeypatch.setattr(edit_cli_module, "validate_input", _raise_os_error) + + result = _run_cli(["validate", "--input", str(path)]) + + assert result.returncode == 1 + assert result.stdout == "" + assert "Error: boom" in result.stderr + + +def test_extraction_help_mentions_editing_commands() -> None: + help_text = build_parser( + availability=ComAvailability(available=False, reason="test") + ).format_help() + + assert "Editing commands:" in help_text + assert "exstruct patch --input book.xlsx --ops ops.json" in help_text + + +def test_main_keeps_legacy_input_on_extraction_path( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + source = tmp_path / "patch.xlsx" + source.write_bytes(b"x") + called: dict[str, Path] = {} + + def _fake_process_excel(*, file_path: Path, **_kwargs: object) -> None: + called["file_path"] = file_path + + monkeypatch.setattr(cli_main_module, "process_excel", _fake_process_excel) + + result = _run_cli([str(source)]) + + assert result.returncode == 0 + assert called["file_path"] == source + assert is_edit_subcommand([str(source)]) is False From dff6126cb084fd8081dcf185613967307643f577 Mon Sep 17 00:00:00 2001 From: harumiWeb Date: Sun, 15 Mar 2026 23:00:23 +0900 Subject: [PATCH 2/3] Address PR #103 review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- dev-docs/specs/editing-cli.md | 2 +- docs/cli.md | 4 +-- src/exstruct/cli/edit.py | 30 +++++++++++++++---- tests/cli/test_edit_cli.py | 55 +++++++++++++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 9 deletions(-) diff --git a/dev-docs/specs/editing-cli.md b/dev-docs/specs/editing-cli.md index 2ff68ae8..19f5d9e0 100644 --- a/dev-docs/specs/editing-cli.md +++ b/dev-docs/specs/editing-cli.md @@ -29,7 +29,7 @@ This document defines the Phase 2 public editing CLI contract. - Shared request flags: - `--sheet` - `--on-conflict {overwrite,skip,rename}` - - `--backend {auto,openpyxl,com}` + - `--backend {auto,com,openpyxl}` - `--auto-formula` - `--dry-run` - `--return-inverse-ops` diff --git a/docs/cli.md b/docs/cli.md index 4582e9da..810e8ab4 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -48,11 +48,11 @@ exstruct validate --input book.xlsx --pretty | Flag | Description | | ---- | ----------- | | `--input PATH` | Existing workbook to edit. | -| `--ops FILE|-` | JSON array of patch ops from a file or stdin. | +| `--ops FILE\|-` | JSON array of patch ops from a file or stdin. | | `--output PATH` | Optional output workbook path. If omitted, the existing default patch output naming applies. | | `--sheet TEXT` | Top-level sheet fallback for patch ops. | | `--on-conflict {overwrite,skip,rename}` | Output conflict policy. | -| `--backend {auto,openpyxl,com}` | Backend selection. | +| `--backend {auto,com,openpyxl}` | Backend selection. | | `--auto-formula` | Treat `=...` values in `set_value` ops as formulas. | | `--dry-run` | Simulate changes without saving. | | `--return-inverse-ops` | Return inverse ops when supported. | diff --git a/src/exstruct/cli/edit.py b/src/exstruct/cli/edit.py index 8fdf5762..fa35f7a7 100644 --- a/src/exstruct/cli/edit.py +++ b/src/exstruct/cli/edit.py @@ -6,12 +6,14 @@ import json from pathlib import Path import sys -from typing import Any +from typing import Any, get_args from pydantic import BaseModel, ValidationError from exstruct.edit import ( MakeRequest, + OnConflictPolicy, + PatchBackend, PatchOp, PatchRequest, coerce_patch_ops, @@ -23,8 +25,21 @@ from exstruct.mcp.validate_input import ValidateInputRequest, validate_input _EDIT_SUBCOMMANDS = frozenset({"patch", "make", "ops", "validate"}) -_ON_CONFLICT_CHOICES = ["overwrite", "skip", "rename"] -_BACKEND_CHOICES = ["auto", "openpyxl", "com"] + + +def _literal_choices(literal_type: object) -> tuple[str, ...]: + """Return argparse choices derived from one string Literal type.""" + + choices: list[str] = [] + for choice in get_args(literal_type): + if not isinstance(choice, str): + raise TypeError("CLI choices must derive from string Literal values.") + choices.append(choice) + return tuple(choices) + + +_ON_CONFLICT_CHOICES = _literal_choices(OnConflictPolicy) +_BACKEND_CHOICES = _literal_choices(PatchBackend) def is_edit_subcommand(argv: list[str]) -> bool: @@ -106,7 +121,10 @@ def run_edit_cli(argv: list[str]) -> int: """Run the edit-subcommand CLI.""" parser = build_edit_parser() - args = parser.parse_args(argv) + try: + args = parser.parse_args(argv) + except SystemExit as exc: + return 0 if exc.code in (None, 0) else 1 handler = getattr(args, "handler", None) if handler is None: parser.print_help() @@ -206,7 +224,7 @@ def _run_patch_command(args: argparse.Namespace) -> int: request_kwargs["out_name"] = args.output.name request = PatchRequest(**request_kwargs) result = patch_workbook(request) - except (OSError, ValidationError, ValueError) as exc: + except (OSError, RuntimeError, ValidationError, ValueError) as exc: _print_error(exc) return 1 @@ -231,7 +249,7 @@ def _run_make_command(args: argparse.Namespace) -> int: backend=args.backend, ) result = make_workbook(request) - except (OSError, ValidationError, ValueError) as exc: + except (OSError, RuntimeError, ValidationError, ValueError) as exc: _print_error(exc) return 1 diff --git a/tests/cli/test_edit_cli.py b/tests/cli/test_edit_cli.py index fa68abf1..3ec04746 100644 --- a/tests/cli/test_edit_cli.py +++ b/tests/cli/test_edit_cli.py @@ -125,6 +125,24 @@ def test_patch_cli_returns_nonzero_for_invalid_ops_json(tmp_path: Path) -> None: assert "Invalid JSON in --ops" in result.stderr +def test_patch_cli_converts_argparse_errors_to_exit_one() -> None: + result = _run_cli(["patch"]) + + assert result.returncode == 1 + assert result.stdout == "" + assert "required" in result.stderr + + +def test_patch_cli_help_keeps_exit_zero() -> None: + result = _run_cli(["patch", "--help"]) + + assert result.returncode == 0 + assert "--input INPUT" in result.stdout + assert "--backend {auto,com,openpyxl}" in result.stdout + assert "--ops OPS" in result.stdout + assert result.stderr == "" + + def test_patch_cli_returns_nonzero_when_patch_result_contains_error( tmp_path: Path, ) -> None: @@ -156,6 +174,26 @@ def test_patch_cli_returns_nonzero_when_patch_result_contains_error( assert payload["error"]["sheet"] == "Missing" +def test_patch_cli_returns_nonzero_when_backend_raises_runtime_error( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + source = tmp_path / "book.xlsx" + ops_path = tmp_path / "ops.json" + _create_workbook(source) + ops_path.write_text("[]", encoding="utf-8") + + def _raise_runtime_error(_request: object) -> object: + raise RuntimeError("backend boom") + + monkeypatch.setattr(edit_cli_module, "patch_workbook", _raise_runtime_error) + + result = _run_cli(["patch", "--input", str(source), "--ops", str(ops_path)]) + + assert result.returncode == 1 + assert result.stdout == "" + assert "Error: backend boom" in result.stderr + + def test_make_cli_creates_workbook_and_returns_json(tmp_path: Path) -> None: output = tmp_path / "created.xlsx" ops_path = tmp_path / "ops.json" @@ -187,6 +225,23 @@ def test_make_cli_creates_workbook_and_returns_json(tmp_path: Path) -> None: assert output.exists() +def test_make_cli_returns_nonzero_when_backend_raises_runtime_error( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + output = tmp_path / "created.xlsx" + + def _raise_runtime_error(_request: object) -> object: + raise RuntimeError("make boom") + + monkeypatch.setattr(edit_cli_module, "make_workbook", _raise_runtime_error) + + result = _run_cli(["make", "--output", str(output)]) + + assert result.returncode == 1 + assert result.stdout == "" + assert "Error: make boom" in result.stderr + + def test_make_cli_defaults_to_empty_ops(tmp_path: Path) -> None: output = tmp_path / "empty.xlsx" From 6db6b16949f4ff251ca459ee6a9f28471355dd7c Mon Sep 17 00:00:00 2001 From: harumiWeb Date: Sun, 15 Mar 2026 23:11:15 +0900 Subject: [PATCH 3/3] Fix CLI top-level sheet fallback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/exstruct/cli/edit.py | 19 ++++++---- tests/cli/test_edit_cli.py | 72 +++++++++++++++++++++++++++++++++++++- 2 files changed, 84 insertions(+), 7 deletions(-) diff --git a/src/exstruct/cli/edit.py b/src/exstruct/cli/edit.py index fa35f7a7..97f3b92f 100644 --- a/src/exstruct/cli/edit.py +++ b/src/exstruct/cli/edit.py @@ -16,11 +16,11 @@ PatchBackend, PatchOp, PatchRequest, - coerce_patch_ops, get_patch_op_schema, list_patch_op_schemas, make_workbook, patch_workbook, + resolve_top_level_sheet_for_payload, ) from exstruct.mcp.validate_input import ValidateInputRequest, validate_input @@ -207,7 +207,7 @@ def _run_patch_command(args: argparse.Namespace) -> int: """Execute the patch subcommand.""" try: - ops = _load_patch_ops(args.ops) + ops = _load_patch_ops(args.ops, sheet=args.sheet) request_kwargs: dict[str, Any] = { "xlsx_path": args.input, "ops": ops, @@ -236,7 +236,7 @@ def _run_make_command(args: argparse.Namespace) -> int: """Execute the make subcommand.""" try: - ops = _load_patch_ops(args.ops) + ops = _load_patch_ops(args.ops, sheet=args.sheet) request = MakeRequest( out_path=args.output, ops=ops, @@ -294,7 +294,7 @@ def _run_validate_command(args: argparse.Namespace) -> int: return 0 if result.is_readable else 1 -def _load_patch_ops(source: str | None) -> list[PatchOp]: +def _load_patch_ops(source: str | None, *, sheet: str | None = None) -> list[PatchOp]: """Load patch ops from a JSON file or stdin.""" if source is None: @@ -302,8 +302,15 @@ def _load_patch_ops(source: str | None) -> list[PatchOp]: payload = _load_json_value(source) if not isinstance(payload, list): raise ValueError("--ops must contain a JSON array.") - normalized_ops = coerce_patch_ops(payload) - return [PatchOp(**op_payload) for op_payload in normalized_ops] + resolved_payload = resolve_top_level_sheet_for_payload( + {"ops": payload, "sheet": sheet} + ) + if not isinstance(resolved_payload, dict): + raise TypeError("Top-level sheet resolution must return a dict payload.") + resolved_ops = resolved_payload.get("ops") + if not isinstance(resolved_ops, list): + raise TypeError("Resolved patch ops payload must contain an ops list.") + return [PatchOp(**op_payload) for op_payload in resolved_ops] def _load_json_value(source: str) -> object: diff --git a/tests/cli/test_edit_cli.py b/tests/cli/test_edit_cli.py index 3ec04746..928648a0 100644 --- a/tests/cli/test_edit_cli.py +++ b/tests/cli/test_edit_cli.py @@ -5,7 +5,7 @@ import json from pathlib import Path -from openpyxl import Workbook +from openpyxl import Workbook, load_workbook from pydantic import BaseModel import pytest @@ -34,6 +34,14 @@ def _create_workbook(path: Path) -> None: workbook.close() +def _read_cell(path: Path, sheet_name: str, cell: str) -> object: + workbook = load_workbook(path) + try: + return workbook[sheet_name][cell].value + finally: + workbook.close() + + def _run_cli(args: list[str], *, stdin_text: str | None = None) -> CliResult: stdout_buffer = io.StringIO() stderr_buffer = io.StringIO() @@ -113,6 +121,35 @@ def test_patch_cli_reads_ops_from_stdin(tmp_path: Path) -> None: assert Path(payload["out_path"]).exists() +def test_patch_cli_applies_top_level_sheet_fallback(tmp_path: Path) -> None: + source = tmp_path / "book.xlsx" + ops_path = tmp_path / "ops.json" + _create_workbook(source) + ops_path.write_text( + json.dumps([{"op": "set_value", "cell": "A1", "value": "fallback"}]), + encoding="utf-8", + ) + + result = _run_cli( + [ + "patch", + "--input", + str(source), + "--ops", + str(ops_path), + "--sheet", + "Sheet1", + "--backend", + "openpyxl", + ] + ) + + assert result.returncode == 0 + payload = json.loads(result.stdout) + assert payload["error"] is None + assert _read_cell(Path(payload["out_path"]), "Sheet1", "A1") == "fallback" + + def test_patch_cli_returns_nonzero_for_invalid_ops_json(tmp_path: Path) -> None: source = tmp_path / "book.xlsx" ops_path = tmp_path / "ops.json" @@ -225,6 +262,39 @@ def test_make_cli_creates_workbook_and_returns_json(tmp_path: Path) -> None: assert output.exists() +def test_make_cli_applies_top_level_sheet_fallback(tmp_path: Path) -> None: + output = tmp_path / "created.xlsx" + ops_path = tmp_path / "ops.json" + ops_path.write_text( + json.dumps( + [ + {"op": "add_sheet", "sheet": "Data"}, + {"op": "set_value", "cell": "A1", "value": "fallback"}, + ] + ), + encoding="utf-8", + ) + + result = _run_cli( + [ + "make", + "--output", + str(output), + "--ops", + str(ops_path), + "--sheet", + "Data", + "--backend", + "openpyxl", + ] + ) + + assert result.returncode == 0 + payload = json.loads(result.stdout) + assert payload["error"] is None + assert _read_cell(output, "Data", "A1") == "fallback" + + def test_make_cli_returns_nonzero_when_backend_raises_runtime_error( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: