diff --git a/dev-docs/adr/ADR-0006-public-edit-api-and-host-boundary.md b/dev-docs/adr/ADR-0006-public-edit-api-and-host-boundary.md new file mode 100644 index 00000000..7b5d3f4e --- /dev/null +++ b/dev-docs/adr/ADR-0006-public-edit-api-and-host-boundary.md @@ -0,0 +1,81 @@ +# ADR-0006: Public Edit API and Host-Owned Safety Boundary + +## Status + +`accepted` + +## Background + +ExStruct already had a capable workbook editing pipeline, but it was reachable +primarily through MCP-facing modules such as `exstruct.mcp.patch_runner` and the +`exstruct_patch` / `exstruct_make` tool handlers. That made normal Python usage +awkward and blurred the boundary between editing behavior and host-owned safety +policy. + +The issue is not only discoverability. The MCP layer also owns path sandboxing, +artifact mirroring, and server execution concerns that should not be mandatory +for ordinary library callers. At the same time, existing patch models, backend +selection rules, and warning/error payloads are already tested and should remain +stable while the public surface is promoted. + +## Decision + +- `exstruct.edit` is adopted as the first-class public Python API for workbook + editing. +- Phase 1 public entry points are `patch_workbook(PatchRequest)` and + `make_workbook(MakeRequest)`. +- The existing patch contract is preserved for Phase 1: + - `PatchOp`, `PatchRequest`, `MakeRequest`, `PatchResult` + - op names + - normalization behavior + - schema-discovery metadata + - backend warning/error payload shape +- MCP remains a host/integration layer. It continues to own: + - `PathPolicy` + - MCP tool input/output mapping + - artifact mirroring + - server defaults, thread offloading, and runtime controls +- Phase 1 may reuse the proven `exstruct.mcp.patch.*` execution pipeline under + the hood while `exstruct.edit` becomes the canonical public import path. + +## Consequences + +- Python callers now have a direct, library-oriented entry point that does not + require MCP-specific path restrictions. +- MCP compatibility remains intact because the old import paths stay available. +- Operation schemas and alias normalization can now be treated as part of the + public editing surface, not only MCP documentation. +- The transition keeps two module trees in play during Phase 1, which is less + clean than a full implementation relocation but materially reduces risk while + the new public surface is established. +- Future phases can move more execution internals under `exstruct.edit` without + reopening the public contract question. + +## Rationale + +- Tests: + - `tests/edit/test_api.py` + - `tests/mcp/patch/test_normalize.py` + - `tests/mcp/test_patch_runner.py` + - `tests/mcp/test_make_runner.py` + - `tests/mcp/patch/test_service.py` + - `tests/mcp/test_tools_handlers.py` +- Code: + - `src/exstruct/edit/__init__.py` + - `src/exstruct/edit/api.py` + - `src/exstruct/edit/service.py` + - `src/exstruct/mcp/patch_runner.py` + - `src/exstruct/mcp/tools.py` +- Related specs: + - `dev-docs/specs/editing-api.md` + - `dev-docs/specs/data-model.md` + - `docs/api.md` + - `docs/mcp.md` + +## Supersedes + +- None + +## Superseded by + +- None diff --git a/dev-docs/adr/README.md b/dev-docs/adr/README.md index caba1817..a3dfe852 100644 --- a/dev-docs/adr/README.md +++ b/dev-docs/adr/README.md @@ -40,3 +40,4 @@ ADRs record what was decided, under which constraints, and which trade-offs were | `ADR-0003` | Output Serialization Omission Policy | `accepted` | `schema` | | `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` | diff --git a/dev-docs/adr/decision-map.md b/dev-docs/adr/decision-map.md index b4db0d33..409243f5 100644 --- a/dev-docs/adr/decision-map.md +++ b/dev-docs/adr/decision-map.md @@ -37,10 +37,20 @@ This document is a human-readable map for navigating ADRs by domain. - `ADR-0004` Patch Backend Selection Policy (`accepted`) - `ADR-0005` PathPolicy Safety Boundary (`accepted`) +- `ADR-0006` Public Edit API and Host-Owned Safety Boundary (`accepted`) ## safety - `ADR-0005` PathPolicy Safety Boundary (`accepted`) +- `ADR-0006` Public Edit API and Host-Owned Safety Boundary (`accepted`) + +## editing + +- `ADR-0006` Public Edit API and Host-Owned Safety Boundary (`accepted`) + +## api + +- `ADR-0006` Public Edit API and Host-Owned Safety Boundary (`accepted`) ## Supersession Relationships diff --git a/dev-docs/adr/index.yaml b/dev-docs/adr/index.yaml index f2044731..90ff25d5 100644 --- a/dev-docs/adr/index.yaml +++ b/dev-docs/adr/index.yaml @@ -67,3 +67,20 @@ adrs: superseded_by: [] related_specs: - docs/mcp.md + - id: ADR-0006 + title: Public Edit API and Host-Owned Safety Boundary + status: accepted + path: dev-docs/adr/ADR-0006-public-edit-api-and-host-boundary.md + primary_domain: editing + domains: + - editing + - api + - mcp + - safety + supersedes: [] + superseded_by: [] + related_specs: + - dev-docs/specs/editing-api.md + - dev-docs/specs/data-model.md + - docs/api.md + - docs/mcp.md diff --git a/dev-docs/architecture/overview.md b/dev-docs/architecture/overview.md index d4c40dd0..5d1874fd 100644 --- a/dev-docs/architecture/overview.md +++ b/dev-docs/architecture/overview.md @@ -29,6 +29,17 @@ exstruct/ io/ serialize.py render/ + edit/ + __init__.py + api.py + chart_types.py + errors.py + service.py + models.py + normalize.py + op_schema.py + specs.py + types.py cli/ main.py ``` @@ -73,9 +84,18 @@ PDF/PNG output (for RAG use cases) CLI entry point +### edit/ + +First-class public workbook editing API + +- `api.py` / `service.py` → public patch/make entry points for Python callers +- `models.py` → public edit request/result models +- `normalize.py` / `specs.py` / `op_schema.py` → public patch-op normalization and schema metadata +- Phase 1 keeps the proven backend execution under `mcp/patch/*` while `edit/` becomes the canonical public import path + ### mcp/patch (Patch Implementation) -Patch functionality is responsibility-separated under `src/exstruct/mcp/patch/`. +MCP editing remains the integration layer around the public edit API. - `patch_runner.py` → compatibility facade for maintaining existing import paths - `patch/internal.py` → internal compatibility layer for patch implementation (non-public) @@ -88,6 +108,14 @@ Patch functionality is responsibility-separated under `src/exstruct/mcp/patch/`. - `patch/normalize.py` / `patch/specs.py` → op normalization and spec metadata - `shared/a1.py` / `shared/output_path.py` → shared utilities for A1 notation and output paths +### mcp/ + +Host-layer responsibilities for MCP and agent tooling + +- `io.py` → `PathPolicy` sandbox boundary +- `tools.py` / `server.py` → tool transport, artifact mirroring, runtime defaults, and thread offloading +- The MCP layer keeps safety policy and host behavior out of the public Python editing API + --- ## Guide for AI Agents diff --git a/dev-docs/specs/data-model.md b/dev-docs/specs/data-model.md index 015b4583..c86cd1af 100644 --- a/dev-docs/specs/data-model.md +++ b/dev-docs/specs/data-model.md @@ -258,16 +258,18 @@ Common: --- -# Appendix A. MCP Patch Models +# Appendix A. Editing Models -The model group used by MCP patch/make operations remains importable from -`exstruct.mcp.patch_runner` for backward compatibility. +The model group used by workbook editing remains importable from both +`exstruct.edit` and `exstruct.mcp.patch_runner` for backward compatibility. The actual locations are as follows. -- Canonical models: `src/exstruct/mcp/patch/models.py` -- Compatibility facade: `src/exstruct/mcp/patch_runner.py` -- Service layer: `src/exstruct/mcp/patch/service.py` +- Primary public import path: `exstruct.edit` / `exstruct.edit.models` +- Current backing implementation module: `exstruct.mcp.patch.models` +- Compatibility facade import path: `exstruct.mcp.patch_runner` +- Public service layer import path: `exstruct.edit.service` +- MCP integration layer import path: `exstruct.mcp.patch.service` Primary models: diff --git a/dev-docs/specs/editing-api.md b/dev-docs/specs/editing-api.md new file mode 100644 index 00000000..3dbfa5bb --- /dev/null +++ b/dev-docs/specs/editing-api.md @@ -0,0 +1,68 @@ +# Editing API Specification + +This document defines the Phase 1 public editing contract exposed from +`exstruct.edit`. + +## Public import path + +- Primary public package: `exstruct.edit` +- Primary functions: + - `patch_workbook(request: PatchRequest) -> PatchResult` + - `make_workbook(request: MakeRequest) -> PatchResult` +- Primary public models: + - `PatchOp` + - `PatchRequest` + - `MakeRequest` + - `PatchResult` + - `PatchDiffItem` + - `PatchErrorDetail` + - `FormulaIssue` + +## Phase 1 guarantees + +- Python callers can edit workbooks through `exstruct.edit` without providing + MCP-specific `PathPolicy` restrictions. +- The patch operation vocabulary, field names, defaults, warnings, and error + payload shapes remain aligned with the existing MCP patch contract. +- `exstruct.edit` exposes the same operation normalization and schema metadata + used by MCP: + - `coerce_patch_ops` + - `resolve_top_level_sheet_for_payload` + - `list_patch_op_schemas` + - `get_patch_op_schema` +- Existing MCP compatibility imports remain valid: + - `exstruct.mcp.patch_runner` + - `exstruct.mcp.patch.normalize` + - `exstruct.mcp.patch.specs` + - `exstruct.mcp.op_schema` + +## Host-only responsibilities + +The following behaviors are not part of the Python editing API contract and +remain owned by MCP / agent hosts: + +- `PathPolicy` root restrictions and deny-glob enforcement +- MCP tool input/output models and transport mapping +- artifact mirroring for MCP hosts +- server-level defaults such as `--on-conflict` +- thread offloading, timeouts, and confirmation flows + +## Current implementation boundary + +- Phase 1 promotes `exstruct.edit` as the canonical public import path. +- The implementation intentionally reuses the existing patch execution pipeline + under `exstruct.mcp.patch.*` to avoid destabilizing the tested backend logic + during the API promotion. +- Contract metadata moved to `exstruct.edit` in Phase 1: + - patch op types + - chart type metadata + - patch op alias/spec metadata + - public op schema discovery + +## Explicit non-goals for Phase 1 + +- No top-level `from exstruct import patch_workbook` export +- No new CLI subcommands +- No op renaming (`set_value` remains the public op name) +- No change to backend selection or fallback policy +- No change to `PatchResult` shape diff --git a/docs/api.md b/docs/api.md index becdb683..e011e454 100644 --- a/docs/api.md +++ b/docs/api.md @@ -9,9 +9,11 @@ This page shows the primary APIs, minimal runnable examples, expected outputs, a - [API Reference](#api-reference) - [TOC](#toc) - [Quick Examples](#quick-examples) + - [Editing API](#editing-api) - [Dependencies](#dependencies) - [Auto-generated API mkdocstrings](#auto-generated-api-mkdocstrings) - [Core functions](#core-functions) + - [Editing functions](#editing-functions) - [Engine and options](#engine-and-options) - [Models](#models) - [Model helpers SheetData / WorkbookData](#model-helpers-sheetdata--workbookdata) @@ -74,6 +76,36 @@ process_excel( # Same as: exstruct input.xlsx --format json --include-backend-metadata --pdf --image --mode standard --pretty --sheets-dir out_sheets > out.json ``` +## Editing API + +Phase 1 exposes workbook editing as a first-class Python package under +`exstruct.edit`. + +```python +from pathlib import Path + +from exstruct.edit import PatchOp, PatchRequest, patch_workbook + +result = patch_workbook( + PatchRequest( + xlsx_path=Path("book.xlsx"), + ops=[PatchOp(op="set_value", sheet="Sheet1", cell="A1", value="updated")], + backend="openpyxl", + ) +) + +print(result.out_path) +print(result.engine) +``` + +Key points: + +- `exstruct.edit` does not require MCP `PathPolicy`. +- `PatchOp`, `PatchRequest`, `MakeRequest`, and `PatchResult` keep the existing + MCP patch contract in Phase 1. +- Use `list_patch_op_schemas()` / `get_patch_op_schema()` to inspect the public + operation schema programmatically. + ## Dependencies - Core extraction: pandas, openpyxl (installed with the package). @@ -142,6 +174,20 @@ Python APIの最新情報は以下の自動生成セクションを参照して show_signature_annotations: true show_root_heading: true +### Editing functions + +::: exstruct.edit.patch_workbook + handler: python + options: + show_signature_annotations: true + show_root_heading: true + +::: exstruct.edit.make_workbook + handler: python + options: + show_signature_annotations: true + show_root_heading: true + ### Engine and options ::: exstruct.engine.ExStructEngine diff --git a/docs/mcp.md b/docs/mcp.md index 90074cda..87dbbd12 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -297,15 +297,21 @@ Example: ### Internal implementation note +Workbook editing now also has a public Python import path under `exstruct.edit`. +MCP remains the host/integration layer and keeps path policy plus transport +concerns outside that public API. + The patch implementation is layered to keep compatibility while enabling refactoring: +- `exstruct.edit`: first-class Python editing API - `exstruct.mcp.patch_runner`: compatibility facade (existing import path) - `exstruct.mcp.patch.service`: patch/make orchestration - `exstruct.mcp.patch.engine.*`: backend execution boundaries (openpyxl/com) - `exstruct.mcp.patch.runtime`: runtime utilities (path/backend selection) - `exstruct.mcp.patch.ops.*`: backend-specific op application entrypoints -This keeps MCP tool I/O stable while allowing internal module separation. +This keeps MCP tool I/O stable while allowing the Python API and host policy to +evolve independently. ## Edit flow (patch) diff --git a/src/exstruct/edit/__init__.py b/src/exstruct/edit/__init__.py new file mode 100644 index 00000000..836d656a --- /dev/null +++ b/src/exstruct/edit/__init__.py @@ -0,0 +1,128 @@ +"""First-class workbook editing API for ExStruct.""" + +from __future__ import annotations + +from .api import make_workbook, patch_workbook +from .chart_types import ( + CHART_TYPE_ALIASES, + CHART_TYPE_TO_COM_ID, + SUPPORTED_CHART_TYPES, + SUPPORTED_CHART_TYPES_CSV, + SUPPORTED_CHART_TYPES_SET, + normalize_chart_type, + resolve_chart_type_id, +) +from .errors import PatchOpError +from .models import ( + AlignmentSnapshot, + BorderSideSnapshot, + BorderSnapshot, + ColumnDimensionSnapshot, + DesignSnapshot, + FillSnapshot, + FontSnapshot, + FormulaIssue, + MakeRequest, + MergeStateSnapshot, + OpenpyxlEngineResult, + OpenpyxlWorksheetProtocol, + PatchDiffItem, + PatchErrorDetail, + PatchOp, + PatchRequest, + PatchResult, + PatchValue, + RowDimensionSnapshot, + XlwingsRangeProtocol, +) +from .normalize import ( + alias_to_canonical_with_conflict_check, + build_missing_sheet_message, + build_patch_op_error_message, + coerce_patch_ops, + normalize_draw_grid_border_range, + normalize_patch_op_aliases, + normalize_top_level_sheet, + parse_patch_op_json, + resolve_top_level_sheet_for_payload, +) +from .op_schema import ( + PatchOpSchema, + build_patch_tool_mini_schema, + get_patch_op_schema, + list_patch_op_schemas, + schema_with_sheet_resolution_rules, +) +from .specs import PATCH_OP_SPECS, PatchOpSpec, get_alias_map_for_op +from .types import ( + FormulaIssueCode, + FormulaIssueLevel, + HorizontalAlignType, + OnConflictPolicy, + PatchBackend, + PatchEngine, + PatchOpType, + PatchStatus, + PatchValueKind, + VerticalAlignType, +) + +__all__ = [ + "AlignmentSnapshot", + "BorderSideSnapshot", + "BorderSnapshot", + "CHART_TYPE_ALIASES", + "CHART_TYPE_TO_COM_ID", + "ColumnDimensionSnapshot", + "DesignSnapshot", + "FillSnapshot", + "FontSnapshot", + "FormulaIssue", + "FormulaIssueCode", + "FormulaIssueLevel", + "HorizontalAlignType", + "MakeRequest", + "MergeStateSnapshot", + "OnConflictPolicy", + "OpenpyxlEngineResult", + "OpenpyxlWorksheetProtocol", + "PATCH_OP_SPECS", + "PatchBackend", + "PatchDiffItem", + "PatchEngine", + "PatchErrorDetail", + "PatchOp", + "PatchOpError", + "PatchOpSchema", + "PatchOpSpec", + "PatchOpType", + "PatchRequest", + "PatchResult", + "PatchStatus", + "PatchValue", + "PatchValueKind", + "RowDimensionSnapshot", + "SUPPORTED_CHART_TYPES", + "SUPPORTED_CHART_TYPES_CSV", + "SUPPORTED_CHART_TYPES_SET", + "VerticalAlignType", + "XlwingsRangeProtocol", + "alias_to_canonical_with_conflict_check", + "build_missing_sheet_message", + "build_patch_op_error_message", + "build_patch_tool_mini_schema", + "coerce_patch_ops", + "get_alias_map_for_op", + "get_patch_op_schema", + "list_patch_op_schemas", + "make_workbook", + "normalize_chart_type", + "normalize_draw_grid_border_range", + "normalize_patch_op_aliases", + "normalize_top_level_sheet", + "parse_patch_op_json", + "patch_workbook", + "resolve_chart_type_id", + "resolve_top_level_sheet_for_payload", + "schema_with_sheet_resolution_rules", +] diff --git a/src/exstruct/edit/api.py b/src/exstruct/edit/api.py new file mode 100644 index 00000000..cdd3efeb --- /dev/null +++ b/src/exstruct/edit/api.py @@ -0,0 +1,14 @@ +"""Public function entry points for workbook editing.""" + +from __future__ import annotations + +from .models import MakeRequest, PatchRequest, PatchResult +from .service import make_workbook, patch_workbook + +__all__ = [ + "make_workbook", + "patch_workbook", + "MakeRequest", + "PatchRequest", + "PatchResult", +] diff --git a/src/exstruct/edit/chart_types.py b/src/exstruct/edit/chart_types.py new file mode 100644 index 00000000..5eff136f --- /dev/null +++ b/src/exstruct/edit/chart_types.py @@ -0,0 +1,59 @@ +"""Chart type constants and COM mappings exposed by the edit API.""" + +from __future__ import annotations + +from typing import Final + +_CHART_TYPE_ENTRIES: Final[tuple[tuple[str, int], ...]] = ( + ("line", 4), + ("column", 51), + ("bar", 57), + ("area", 1), + ("pie", 5), + ("doughnut", -4120), + ("scatter", -4169), + ("radar", -4151), +) + +SUPPORTED_CHART_TYPES: Final[tuple[str, ...]] = tuple(t for t, _ in _CHART_TYPE_ENTRIES) +CHART_TYPE_TO_COM_ID: Final[dict[str, int]] = dict(_CHART_TYPE_ENTRIES) + +CHART_TYPE_ALIASES: Final[dict[str, str]] = { + "column_clustered": "column", + "bar_clustered": "bar", + "xy_scatter": "scatter", + "donut": "doughnut", +} + +SUPPORTED_CHART_TYPES_SET: Final[frozenset[str]] = frozenset(SUPPORTED_CHART_TYPES) +SUPPORTED_CHART_TYPES_CSV: Final[str] = ", ".join(SUPPORTED_CHART_TYPES) + + +def normalize_chart_type(chart_type: str) -> str | None: + """Normalize chart type input to a canonical key.""" + + candidate = chart_type.strip().lower() + canonical = CHART_TYPE_ALIASES.get(candidate, candidate) + if canonical in SUPPORTED_CHART_TYPES_SET: + return canonical + return None + + +def resolve_chart_type_id(chart_type: str) -> int | None: + """Resolve canonical chart type key to Excel COM ChartType ID.""" + + normalized = normalize_chart_type(chart_type) + if normalized is None: + return None + return CHART_TYPE_TO_COM_ID[normalized] + + +__all__ = [ + "CHART_TYPE_ALIASES", + "CHART_TYPE_TO_COM_ID", + "SUPPORTED_CHART_TYPES", + "SUPPORTED_CHART_TYPES_CSV", + "SUPPORTED_CHART_TYPES_SET", + "normalize_chart_type", + "resolve_chart_type_id", +] diff --git a/src/exstruct/edit/errors.py b/src/exstruct/edit/errors.py new file mode 100644 index 00000000..50879f81 --- /dev/null +++ b/src/exstruct/edit/errors.py @@ -0,0 +1,7 @@ +"""Public error types for workbook editing.""" + +from __future__ import annotations + +from exstruct.mcp.patch.ops.common import PatchOpError + +__all__ = ["PatchOpError"] diff --git a/src/exstruct/edit/models.py b/src/exstruct/edit/models.py new file mode 100644 index 00000000..b0dc3cbf --- /dev/null +++ b/src/exstruct/edit/models.py @@ -0,0 +1,53 @@ +"""Public editing models. + +Phase 1 keeps the proven model implementation in the existing patch module while +promoting `exstruct.edit` as the canonical public import path. +""" + +from __future__ import annotations + +from exstruct.mcp.patch.models import ( + AlignmentSnapshot, + BorderSideSnapshot, + BorderSnapshot, + ColumnDimensionSnapshot, + DesignSnapshot, + FillSnapshot, + FontSnapshot, + FormulaIssue, + MakeRequest, + MergeStateSnapshot, + OpenpyxlEngineResult, + OpenpyxlWorksheetProtocol, + PatchDiffItem, + PatchErrorDetail, + PatchOp, + PatchRequest, + PatchResult, + PatchValue, + RowDimensionSnapshot, + XlwingsRangeProtocol, +) + +__all__ = [ + "AlignmentSnapshot", + "BorderSideSnapshot", + "BorderSnapshot", + "ColumnDimensionSnapshot", + "DesignSnapshot", + "FillSnapshot", + "FontSnapshot", + "FormulaIssue", + "MakeRequest", + "MergeStateSnapshot", + "OpenpyxlEngineResult", + "OpenpyxlWorksheetProtocol", + "PatchDiffItem", + "PatchErrorDetail", + "PatchOp", + "PatchRequest", + "PatchResult", + "PatchValue", + "RowDimensionSnapshot", + "XlwingsRangeProtocol", +] diff --git a/src/exstruct/edit/normalize.py b/src/exstruct/edit/normalize.py new file mode 100644 index 00000000..739d657a --- /dev/null +++ b/src/exstruct/edit/normalize.py @@ -0,0 +1,217 @@ +"""Normalization helpers for public and MCP patch-op payloads.""" + +from __future__ import annotations + +from collections.abc import Sequence +import json +from typing import Any, cast + +from exstruct.mcp.shared.a1 import parse_range_geometry + +from .specs import get_alias_map_for_op + + +def coerce_patch_ops(ops_data: Sequence[object]) -> list[dict[str, Any]]: + """Normalize patch operations payload for public and MCP callers.""" + + normalized_ops: list[dict[str, Any]] = [] + for index, raw_op in enumerate(ops_data): + parsed_op = _coerce_patch_op_mapping(raw_op, index=index) + normalized_ops.append(normalize_patch_op_aliases(parsed_op, index=index)) + return normalized_ops + + +def _coerce_patch_op_mapping(raw_op: object, *, index: int) -> dict[str, Any]: + """Coerce one raw patch operation into a normalized mapping.""" + + if isinstance(raw_op, dict): + return dict(raw_op) + if isinstance(raw_op, str): + return parse_patch_op_json(raw_op, index=index) + raise ValueError( + build_patch_op_error_message( + index, + "patch op must be an object or JSON string, " + f"got {type(raw_op).__name__}: {raw_op!r}", + ) + ) + + +def resolve_top_level_sheet_for_payload(data: object) -> object: + """Resolve top-level sheet default into operation dict payloads.""" + + if not isinstance(data, dict): + return data + ops_raw = data.get("ops") + if not isinstance(ops_raw, list): + return data + top_level_sheet = normalize_top_level_sheet(data.get("sheet")) + resolved_ops: list[dict[str, Any]] = [] + for index, op_raw in enumerate(ops_raw): + op_copy = normalize_patch_op_aliases( + _coerce_patch_op_mapping(op_raw, index=index), index=index + ) + op_name_raw = op_copy.get("op") + op_name = op_name_raw if isinstance(op_name_raw, str) else "" + op_sheet = op_copy.get("sheet") + if op_name == "add_sheet": + if op_copy.get("sheet") is None: + raise ValueError( + build_missing_sheet_message(index=index, op_name="add_sheet") + ) + resolved_ops.append(op_copy) + continue + if op_sheet is None: + if top_level_sheet is None: + raise ValueError( + build_missing_sheet_message(index=index, op_name=op_name) + ) + op_copy["sheet"] = top_level_sheet + resolved_ops.append(op_copy) + payload = dict(data) + payload["ops"] = resolved_ops + if top_level_sheet is not None: + payload["sheet"] = top_level_sheet + return payload + + +def normalize_patch_op_aliases( + op_data: dict[str, Any], *, index: int +) -> dict[str, Any]: + """Normalize MCP-friendly aliases to canonical patch operation fields.""" + + normalized = dict(op_data) + op_name = normalized.get("op") + if not isinstance(op_name, str): + return normalized + alias_map = get_alias_map_for_op(op_name) + for alias, canonical in alias_map.items(): + alias_to_canonical_with_conflict_check( + normalized, + index=index, + alias=alias, + canonical=canonical, + op_name=op_name, + ) + normalize_draw_grid_border_range(normalized, index=index) + return normalized + + +def alias_to_canonical_with_conflict_check( + op_data: dict[str, Any], + *, + index: int, + alias: str, + canonical: str, + op_name: str, +) -> None: + """Map alias field to canonical field when operation type matches.""" + + if op_data.get("op") != op_name or alias not in op_data: + return + alias_value = op_data[alias] + canonical_value = op_data.get(canonical) + if canonical in op_data: + if canonical_value != alias_value: + raise ValueError( + build_patch_op_error_message( + index, + f"conflicting fields: '{canonical}' and alias '{alias}'", + ) + ) + else: + op_data[canonical] = alias_value + del op_data[alias] + + +def normalize_draw_grid_border_range(op_data: dict[str, Any], *, index: int) -> None: + """Convert draw_grid_border range shorthand to base/size fields.""" + + if op_data.get("op") != "draw_grid_border" or "range" not in op_data: + return + if "base_cell" in op_data or "row_count" in op_data or "col_count" in op_data: + raise ValueError( + build_patch_op_error_message( + index, + "draw_grid_border does not allow mixing 'range' with 'base_cell/row_count/col_count'", + ) + ) + range_ref = op_data.get("range") + if not isinstance(range_ref, str): + raise ValueError( + build_patch_op_error_message( + index, "draw_grid_border range must be a string A1 range" + ) + ) + try: + start, row_count, col_count = parse_range_geometry(range_ref) + except ValueError as exc: + raise ValueError( + build_patch_op_error_message( + index, "draw_grid_border range must be like 'A1:C3'" + ) + ) from exc + op_data["base_cell"] = start + op_data["row_count"] = row_count + op_data["col_count"] = col_count + del op_data["range"] + + +def parse_patch_op_json(raw_op: str, *, index: int) -> dict[str, Any]: + """Parse a JSON string patch operation into object form.""" + + text = raw_op.strip() + if not text: + raise ValueError(build_patch_op_error_message(index, "empty string")) + try: + parsed = json.loads(text) + except json.JSONDecodeError as exc: + raise ValueError(build_patch_op_error_message(index, "invalid JSON")) from exc + if not isinstance(parsed, dict): + raise ValueError( + build_patch_op_error_message(index, "JSON value must be an object") + ) + return cast(dict[str, Any], parsed) + + +def normalize_top_level_sheet(value: object) -> str | None: + """Normalize optional top-level sheet text.""" + + if not isinstance(value, str): + return None + candidate = value.strip() + return candidate or None + + +def build_missing_sheet_message(*, index: int, op_name: str) -> str: + """Build self-healing error for unresolved sheet selection.""" + + target_op = op_name or "" + return ( + f"ops[{index}] ({target_op}) is missing sheet. " + "Set op.sheet, or set top-level sheet for non-add_sheet ops. " + "For add_sheet, op.sheet (or alias name) is required." + ) + + +def build_patch_op_error_message(index: int, reason: str) -> str: + """Build a consistent validation message for invalid patch ops.""" + + example = '{"op":"set_value","sheet":"Sheet1","cell":"A1","value":"sample"}' + return ( + f"Invalid patch operation at ops[{index}]: {reason}. " + f"Use object form like {example}." + ) + + +__all__ = [ + "alias_to_canonical_with_conflict_check", + "build_missing_sheet_message", + "build_patch_op_error_message", + "coerce_patch_ops", + "normalize_draw_grid_border_range", + "normalize_patch_op_aliases", + "normalize_top_level_sheet", + "parse_patch_op_json", + "resolve_top_level_sheet_for_payload", +] diff --git a/src/exstruct/edit/op_schema.py b/src/exstruct/edit/op_schema.py new file mode 100644 index 00000000..25f771f4 --- /dev/null +++ b/src/exstruct/edit/op_schema.py @@ -0,0 +1,458 @@ +"""Human-readable patch operation schema metadata for public callers.""" + +from __future__ import annotations + +from typing import Any, get_args + +from pydantic import BaseModel, Field + +from .types import PatchOpType + + +class PatchOpSchema(BaseModel): + """Mini schema metadata for a patch operation.""" + + op: PatchOpType + description: str + required: list[str] = Field(default_factory=list) + optional: list[str] = Field(default_factory=list) + constraints: list[str] = Field(default_factory=list) + example: dict[str, Any] + aliases: dict[str, str] = Field(default_factory=dict) + + +def list_patch_op_schemas() -> list[PatchOpSchema]: + """Return patch operation schemas in canonical op order.""" + + ordered_ops = list(get_args(PatchOpType)) + return [_PATCH_OP_SCHEMA_BY_NAME[op] for op in ordered_ops] + + +def get_patch_op_schema(op: str) -> PatchOpSchema | None: + """Return schema for one patch op name.""" + + return _PATCH_OP_SCHEMA_BY_NAME.get(op) + + +def build_patch_tool_mini_schema() -> str: + """Build a human-readable mini schema section for patch docs.""" + + lines: list[str] = [] + lines.append("Mini op schema (required/optional/constraints/example/aliases):") + lines.append( + "Sheet resolution: non-add_sheet ops allow top-level sheet fallback; " + "op.sheet overrides top-level sheet." + ) + for schema in list_patch_op_schemas(): + display_schema = schema_with_sheet_resolution_rules(schema) + lines.append(f"- {schema.op}: {schema.description}") + lines.append( + " required: " + + ( + ", ".join(display_schema.required) + if display_schema.required + else "(none)" + ) + ) + lines.append( + " optional: " + + ( + ", ".join(display_schema.optional) + if display_schema.optional + else "(none)" + ) + ) + lines.append( + " constraints: " + + ( + ", ".join(display_schema.constraints) + if display_schema.constraints + else "(none)" + ) + ) + lines.append(f" example: {display_schema.example}") + lines.append( + " aliases: " + + ( + ", ".join( + f"{alias} -> {canonical}" + for alias, canonical in sorted(display_schema.aliases.items()) + ) + if display_schema.aliases + else "(none)" + ) + ) + return "\n".join(lines) + + +def schema_with_sheet_resolution_rules(schema: PatchOpSchema) -> PatchOpSchema: + """Return display schema with top-level sheet resolution notes.""" + + if "sheet" not in schema.required: + return schema + updated_required = [ + ( + "sheet (or top-level sheet)" + if item == "sheet" and schema.op != "add_sheet" + else item + ) + for item in schema.required + ] + updated_constraints = list(schema.constraints) + if schema.op == "add_sheet": + updated_constraints.append("top-level sheet is not used for add_sheet") + else: + updated_constraints.append( + "op.sheet overrides top-level sheet when both are set" + ) + return schema.model_copy( + update={"required": updated_required, "constraints": updated_constraints} + ) + + +_PATCH_OP_SCHEMA_BY_NAME: dict[str, PatchOpSchema] = { + "set_value": PatchOpSchema( + op="set_value", + description="Set a scalar value to one cell.", + required=["sheet", "cell", "value"], + optional=[], + constraints=[ + "cell target only", + "use auto_formula=true to allow values starting with '='", + ], + example={"op": "set_value", "sheet": "Sheet1", "cell": "A1", "value": "Hello"}, + ), + "set_formula": PatchOpSchema( + op="set_formula", + description="Set one formula string to one cell.", + required=["sheet", "cell", "formula"], + optional=[], + constraints=["formula must start with '='"], + example={ + "op": "set_formula", + "sheet": "Sheet1", + "cell": "B2", + "formula": "=SUM(B1:B10)", + }, + ), + "add_sheet": PatchOpSchema( + op="add_sheet", + description="Add a new worksheet by name.", + required=["sheet"], + optional=[], + constraints=["sheet name must be unique in workbook"], + example={"op": "add_sheet", "sheet": "Data"}, + aliases={"name": "sheet"}, + ), + "set_range_values": PatchOpSchema( + op="set_range_values", + description="Set a 2D values matrix to a rectangular range.", + required=["sheet", "range", "values"], + optional=[], + constraints=["values shape must match range rows x cols"], + example={ + "op": "set_range_values", + "sheet": "Sheet1", + "range": "A1:B2", + "values": [[1, 2], [3, 4]], + }, + ), + "fill_formula": PatchOpSchema( + op="fill_formula", + description="Fill a base formula across target range.", + required=["sheet", "range", "base_cell", "formula"], + optional=[], + constraints=["formula must start with '='"], + example={ + "op": "fill_formula", + "sheet": "Sheet1", + "range": "C2:C10", + "base_cell": "C2", + "formula": "=A2+B2", + }, + ), + "set_value_if": PatchOpSchema( + op="set_value_if", + description="Set value when current value matches expected.", + required=["sheet", "cell", "expected", "value"], + optional=[], + constraints=["no-op when expected mismatch"], + example={ + "op": "set_value_if", + "sheet": "Sheet1", + "cell": "A1", + "expected": "old", + "value": "new", + }, + ), + "set_formula_if": PatchOpSchema( + op="set_formula_if", + description="Set formula when current value matches expected.", + required=["sheet", "cell", "expected", "formula"], + optional=[], + constraints=["formula must start with '='", "no-op when expected mismatch"], + example={ + "op": "set_formula_if", + "sheet": "Sheet1", + "cell": "C5", + "expected": 0, + "formula": "=A5+B5", + }, + ), + "draw_grid_border": PatchOpSchema( + op="draw_grid_border", + description="Draw thin black borders for a rectangular region.", + required=["sheet", "base_cell", "row_count", "col_count"], + optional=[], + constraints=["row_count > 0", "col_count > 0", "or use range shorthand alias"], + example={ + "op": "draw_grid_border", + "sheet": "Sheet1", + "base_cell": "A1", + "row_count": 5, + "col_count": 4, + }, + aliases={"range": "base_cell + row_count + col_count"}, + ), + "set_bold": PatchOpSchema( + op="set_bold", + description="Apply bold font to one cell or one range.", + required=["sheet"], + optional=["cell", "range", "bold"], + constraints=["exactly one of cell or range"], + example={"op": "set_bold", "sheet": "Sheet1", "range": "A1:D1", "bold": True}, + ), + "set_font_size": PatchOpSchema( + op="set_font_size", + description="Apply font size to one cell or one range.", + required=["sheet", "font_size"], + optional=["cell", "range"], + constraints=["exactly one of cell or range", "font_size > 0"], + example={ + "op": "set_font_size", + "sheet": "Sheet1", + "cell": "A1", + "font_size": 14, + }, + ), + "set_font_color": PatchOpSchema( + op="set_font_color", + description="Apply font color to one cell or one range.", + required=["sheet", "color"], + optional=["cell", "range"], + constraints=[ + "exactly one of cell or range", + "hex color (#RRGGBB or #AARRGGBB)", + ], + example={ + "op": "set_font_color", + "sheet": "Sheet1", + "range": "A1:D1", + "color": "#1F4E79", + }, + ), + "set_fill_color": PatchOpSchema( + op="set_fill_color", + description="Apply fill color to one cell or one range.", + required=["sheet", "fill_color"], + optional=["cell", "range"], + constraints=[ + "exactly one of cell or range", + "hex color (#RRGGBB or #AARRGGBB)", + ], + example={ + "op": "set_fill_color", + "sheet": "Sheet1", + "range": "A1:D1", + "fill_color": "#D9E1F2", + }, + aliases={"color": "fill_color"}, + ), + "set_dimensions": PatchOpSchema( + op="set_dimensions", + description="Set row height and/or column width.", + required=["sheet"], + optional=["rows", "columns", "row_height", "column_width"], + constraints=[ + "at least one of row_height or column_width", + "row_height > 0, column_width > 0", + ], + example={ + "op": "set_dimensions", + "sheet": "Sheet1", + "rows": [1, 2], + "row_height": 22, + "columns": ["A", "B"], + "column_width": 18, + }, + aliases={ + "row": "rows", + "col": "columns", + "height": "row_height", + "width": "column_width", + }, + ), + "auto_fit_columns": PatchOpSchema( + op="auto_fit_columns", + description="Auto-fit column widths with optional width bounds.", + required=["sheet"], + optional=["columns", "min_width", "max_width"], + constraints=[ + "columns optional (uses used columns when omitted)", + "min_width > 0, max_width > 0 when provided", + "min_width <= max_width when both are provided", + ], + example={ + "op": "auto_fit_columns", + "sheet": "Sheet1", + "columns": ["A", 2], + "min_width": 8, + "max_width": 40, + }, + ), + "merge_cells": PatchOpSchema( + op="merge_cells", + description="Merge one rectangular range.", + required=["sheet", "range"], + optional=[], + constraints=["range must be rectangular"], + example={"op": "merge_cells", "sheet": "Sheet1", "range": "A1:C1"}, + ), + "unmerge_cells": PatchOpSchema( + op="unmerge_cells", + description="Unmerge merged cells intersecting range.", + required=["sheet", "range"], + optional=[], + constraints=["range must be rectangular"], + example={"op": "unmerge_cells", "sheet": "Sheet1", "range": "A1:C1"}, + ), + "set_alignment": PatchOpSchema( + op="set_alignment", + description="Set alignment flags to one cell or one range.", + required=["sheet"], + optional=["cell", "range", "horizontal_align", "vertical_align", "wrap_text"], + constraints=[ + "exactly one of cell or range", + "specify at least one alignment field", + ], + example={ + "op": "set_alignment", + "sheet": "Sheet1", + "range": "A1:D1", + "horizontal_align": "center", + "vertical_align": "center", + "wrap_text": True, + }, + aliases={"horizontal": "horizontal_align", "vertical": "vertical_align"}, + ), + "set_style": PatchOpSchema( + op="set_style", + description="Apply multiple style attributes in one op.", + required=["sheet"], + optional=[ + "cell", + "range", + "bold", + "font_size", + "color", + "fill_color", + "horizontal_align", + "vertical_align", + "wrap_text", + ], + constraints=[ + "exactly one of cell or range", + "specify at least one style field", + "font_size > 0", + "target cell count <= style limit", + ], + example={ + "op": "set_style", + "sheet": "Sheet1", + "range": "A1:D1", + "bold": True, + "color": "#FFFFFF", + "fill_color": "#1F3864", + "horizontal_align": "center", + }, + ), + "apply_table_style": PatchOpSchema( + op="apply_table_style", + description="Create table and apply Excel table style.", + required=["sheet", "range", "style"], + optional=["table_name"], + constraints=[ + "range must include header row", + "table name must be unique when provided", + "range must not intersect existing table", + ], + example={ + "op": "apply_table_style", + "sheet": "Sheet1", + "range": "A1:D11", + "style": "TableStyleMedium9", + "table_name": "SalesTable", + }, + ), + "create_chart": PatchOpSchema( + op="create_chart", + description="Create a new chart on a worksheet (COM backend only).", + required=["sheet", "chart_type", "data_range", "anchor_cell"], + optional=[ + "category_range", + "chart_name", + "width", + "height", + "titles_from_data", + "series_from_rows", + "chart_title", + "x_axis_title", + "y_axis_title", + ], + constraints=[ + "chart_type in {'line','column','bar','area','pie','doughnut','scatter','radar'}", + "data_range accepts A1 range string or list[str] for multi-series input", + "data_range/category_range allow sheet-qualified ranges like 'Sheet2!A1:B10'", + "width/height must be > 0 when provided", + "chart_name must be unique in sheet when provided", + "backend='openpyxl' is not supported", + ], + example={ + "op": "create_chart", + "sheet": "Sheet1", + "chart_type": "line", + "data_range": ["A2:A10", "C2:C10", "D2:D10"], + "category_range": "B2:B10", + "anchor_cell": "E2", + "chart_name": "SalesTrend", + "width": 360, + "height": 220, + "titles_from_data": True, + "series_from_rows": False, + "chart_title": "Sales trend", + "x_axis_title": "Month", + "y_axis_title": "Amount", + }, + ), + "restore_design_snapshot": PatchOpSchema( + op="restore_design_snapshot", + description="Internal inverse op to restore style snapshot.", + required=["sheet", "design_snapshot"], + optional=[], + constraints=["internal use for inverse operations"], + example={ + "op": "restore_design_snapshot", + "sheet": "Sheet1", + "design_snapshot": {"range": "A1:A1", "cells": []}, + }, + ), +} + + +__all__ = [ + "PatchOpSchema", + "build_patch_tool_mini_schema", + "get_patch_op_schema", + "list_patch_op_schemas", + "schema_with_sheet_resolution_rules", +] diff --git a/src/exstruct/edit/service.py b/src/exstruct/edit/service.py new file mode 100644 index 00000000..e116b167 --- /dev/null +++ b/src/exstruct/edit/service.py @@ -0,0 +1,22 @@ +"""Public workbook editing service wrappers.""" + +from __future__ import annotations + +from exstruct.mcp.patch.service import run_make, run_patch + +from .models import MakeRequest, PatchRequest, PatchResult + + +def patch_workbook(request: PatchRequest) -> PatchResult: + """Edit an existing workbook without MCP path policy enforcement.""" + + return run_patch(request, policy=None) + + +def make_workbook(request: MakeRequest) -> PatchResult: + """Create a new workbook and apply initial patch operations.""" + + return run_make(request, policy=None) + + +__all__ = ["make_workbook", "patch_workbook"] diff --git a/src/exstruct/edit/specs.py b/src/exstruct/edit/specs.py new file mode 100644 index 00000000..30e7cd89 --- /dev/null +++ b/src/exstruct/edit/specs.py @@ -0,0 +1,64 @@ +"""Patch-op specification metadata exposed from the public edit API.""" + +from __future__ import annotations + +from typing import Final, cast + +from pydantic import BaseModel, Field + +from .types import PatchOpType + + +class PatchOpSpec(BaseModel): + """Specification metadata used by patch-op normalization.""" + + op: PatchOpType + aliases: dict[str, str] = Field(default_factory=dict) + + +PATCH_OP_SPECS: Final[dict[PatchOpType, PatchOpSpec]] = { + "set_value": PatchOpSpec(op="set_value"), + "set_formula": PatchOpSpec(op="set_formula"), + "add_sheet": PatchOpSpec(op="add_sheet", aliases={"name": "sheet"}), + "set_range_values": PatchOpSpec(op="set_range_values"), + "fill_formula": PatchOpSpec(op="fill_formula"), + "set_value_if": PatchOpSpec(op="set_value_if"), + "set_formula_if": PatchOpSpec(op="set_formula_if"), + "draw_grid_border": PatchOpSpec(op="draw_grid_border"), + "set_bold": PatchOpSpec(op="set_bold"), + "set_font_size": PatchOpSpec(op="set_font_size"), + "set_font_color": PatchOpSpec(op="set_font_color"), + "set_fill_color": PatchOpSpec(op="set_fill_color", aliases={"color": "fill_color"}), + "set_dimensions": PatchOpSpec( + op="set_dimensions", + aliases={ + "row": "rows", + "col": "columns", + "height": "row_height", + "width": "column_width", + }, + ), + "auto_fit_columns": PatchOpSpec(op="auto_fit_columns"), + "merge_cells": PatchOpSpec(op="merge_cells"), + "unmerge_cells": PatchOpSpec(op="unmerge_cells"), + "set_alignment": PatchOpSpec( + op="set_alignment", + aliases={"horizontal": "horizontal_align", "vertical": "vertical_align"}, + ), + "set_style": PatchOpSpec(op="set_style"), + "apply_table_style": PatchOpSpec(op="apply_table_style"), + "create_chart": PatchOpSpec(op="create_chart"), + "restore_design_snapshot": PatchOpSpec(op="restore_design_snapshot"), +} + + +def get_alias_map_for_op(op_name: str) -> dict[str, str]: + """Return alias mapping for one operation name.""" + + if op_name not in PATCH_OP_SPECS: + return {} + spec = PATCH_OP_SPECS[cast(PatchOpType, op_name)] + return dict(spec.aliases) + + +__all__ = ["PATCH_OP_SPECS", "PatchOpSpec", "get_alias_map_for_op"] diff --git a/src/exstruct/edit/types.py b/src/exstruct/edit/types.py new file mode 100644 index 00000000..01fa8185 --- /dev/null +++ b/src/exstruct/edit/types.py @@ -0,0 +1,69 @@ +"""Literal type aliases used by the public workbook editing contract.""" + +from __future__ import annotations + +from typing import Literal + +PatchOpType = Literal[ + "set_value", + "set_formula", + "add_sheet", + "set_range_values", + "fill_formula", + "set_value_if", + "set_formula_if", + "draw_grid_border", + "set_bold", + "set_font_size", + "set_font_color", + "set_fill_color", + "set_dimensions", + "auto_fit_columns", + "merge_cells", + "unmerge_cells", + "set_alignment", + "set_style", + "apply_table_style", + "create_chart", + "restore_design_snapshot", +] +PatchStatus = Literal["applied", "skipped"] +PatchValueKind = Literal["value", "formula", "sheet", "style", "dimension", "chart"] +PatchBackend = Literal["auto", "com", "openpyxl"] +PatchEngine = Literal["com", "openpyxl"] +OnConflictPolicy = Literal["overwrite", "skip", "rename"] +FormulaIssueLevel = Literal["warning", "error"] +FormulaIssueCode = Literal[ + "invalid_token", + "ref_error", + "name_error", + "div0_error", + "value_error", + "na_error", + "circular_ref_suspected", +] + +HorizontalAlignType = Literal[ + "general", + "left", + "center", + "right", + "fill", + "justify", + "centerContinuous", + "distributed", +] +VerticalAlignType = Literal["top", "center", "bottom", "justify", "distributed"] + +__all__ = [ + "FormulaIssueCode", + "FormulaIssueLevel", + "HorizontalAlignType", + "OnConflictPolicy", + "PatchBackend", + "PatchEngine", + "PatchOpType", + "PatchStatus", + "PatchValueKind", + "VerticalAlignType", +] diff --git a/src/exstruct/mcp/op_schema.py b/src/exstruct/mcp/op_schema.py index 61a5dee4..b96d4d4d 100644 --- a/src/exstruct/mcp/op_schema.py +++ b/src/exstruct/mcp/op_schema.py @@ -1,444 +1,17 @@ from __future__ import annotations -from typing import Any, get_args - -from pydantic import BaseModel, Field - -from .patch.types import PatchOpType - - -class PatchOpSchema(BaseModel): - """Mini schema metadata for a patch operation.""" - - op: PatchOpType - description: str - required: list[str] = Field(default_factory=list) - optional: list[str] = Field(default_factory=list) - constraints: list[str] = Field(default_factory=list) - example: dict[str, Any] - aliases: dict[str, str] = Field(default_factory=dict) - - -def list_patch_op_schemas() -> list[PatchOpSchema]: - """Return patch operation schemas in canonical op order.""" - ordered_ops = list(get_args(PatchOpType)) - return [_PATCH_OP_SCHEMA_BY_NAME[op] for op in ordered_ops] - - -def get_patch_op_schema(op: str) -> PatchOpSchema | None: - """Return schema for one patch op name.""" - schema = _PATCH_OP_SCHEMA_BY_NAME.get(op) - return schema - - -def build_patch_tool_mini_schema() -> str: - """Build a human-readable mini schema section for exstruct_patch doc.""" - lines: list[str] = [] - lines.append("Mini op schema (required/optional/constraints/example/aliases):") - lines.append( - "Sheet resolution: non-add_sheet ops allow top-level sheet fallback; " - "op.sheet overrides top-level sheet." - ) - for schema in list_patch_op_schemas(): - display_schema = schema_with_sheet_resolution_rules(schema) - lines.append(f"- {schema.op}: {schema.description}") - lines.append( - " required: " - + ( - ", ".join(display_schema.required) - if display_schema.required - else "(none)" - ) - ) - lines.append( - " optional: " - + ( - ", ".join(display_schema.optional) - if display_schema.optional - else "(none)" - ) - ) - lines.append( - " constraints: " - + ( - ", ".join(display_schema.constraints) - if display_schema.constraints - else "(none)" - ) - ) - lines.append(f" example: {display_schema.example}") - lines.append( - " aliases: " - + ( - ", ".join( - f"{alias} -> {canonical}" - for alias, canonical in sorted(display_schema.aliases.items()) - ) - if display_schema.aliases - else "(none)" - ) - ) - return "\n".join(lines) - - -def schema_with_sheet_resolution_rules(schema: PatchOpSchema) -> PatchOpSchema: - """Return display schema with top-level sheet resolution notes.""" - if "sheet" not in schema.required: - return schema - updated_required = [ - ( - "sheet (or top-level sheet)" - if item == "sheet" and schema.op != "add_sheet" - else item - ) - for item in schema.required - ] - updated_constraints = list(schema.constraints) - if schema.op == "add_sheet": - updated_constraints.append("top-level sheet is not used for add_sheet") - else: - updated_constraints.append( - "op.sheet overrides top-level sheet when both are set" - ) - return schema.model_copy( - update={"required": updated_required, "constraints": updated_constraints} - ) - - -_PATCH_OP_SCHEMA_BY_NAME: dict[str, PatchOpSchema] = { - "set_value": PatchOpSchema( - op="set_value", - description="Set a scalar value to one cell.", - required=["sheet", "cell", "value"], - optional=[], - constraints=[ - "cell target only", - "use auto_formula=true to allow values starting with '='", - ], - example={"op": "set_value", "sheet": "Sheet1", "cell": "A1", "value": "Hello"}, - ), - "set_formula": PatchOpSchema( - op="set_formula", - description="Set one formula string to one cell.", - required=["sheet", "cell", "formula"], - optional=[], - constraints=["formula must start with '='"], - example={ - "op": "set_formula", - "sheet": "Sheet1", - "cell": "B2", - "formula": "=SUM(B1:B10)", - }, - ), - "add_sheet": PatchOpSchema( - op="add_sheet", - description="Add a new worksheet by name.", - required=["sheet"], - optional=[], - constraints=["sheet name must be unique in workbook"], - example={"op": "add_sheet", "sheet": "Data"}, - aliases={"name": "sheet"}, - ), - "set_range_values": PatchOpSchema( - op="set_range_values", - description="Set a 2D values matrix to a rectangular range.", - required=["sheet", "range", "values"], - optional=[], - constraints=["values shape must match range rows x cols"], - example={ - "op": "set_range_values", - "sheet": "Sheet1", - "range": "A1:B2", - "values": [[1, 2], [3, 4]], - }, - ), - "fill_formula": PatchOpSchema( - op="fill_formula", - description="Fill a base formula across target range.", - required=["sheet", "range", "base_cell", "formula"], - optional=[], - constraints=["formula must start with '='"], - example={ - "op": "fill_formula", - "sheet": "Sheet1", - "range": "C2:C10", - "base_cell": "C2", - "formula": "=A2+B2", - }, - ), - "set_value_if": PatchOpSchema( - op="set_value_if", - description="Set value when current value matches expected.", - required=["sheet", "cell", "expected", "value"], - optional=[], - constraints=["no-op when expected mismatch"], - example={ - "op": "set_value_if", - "sheet": "Sheet1", - "cell": "A1", - "expected": "old", - "value": "new", - }, - ), - "set_formula_if": PatchOpSchema( - op="set_formula_if", - description="Set formula when current value matches expected.", - required=["sheet", "cell", "expected", "formula"], - optional=[], - constraints=["formula must start with '='", "no-op when expected mismatch"], - example={ - "op": "set_formula_if", - "sheet": "Sheet1", - "cell": "C5", - "expected": 0, - "formula": "=A5+B5", - }, - ), - "draw_grid_border": PatchOpSchema( - op="draw_grid_border", - description="Draw thin black borders for a rectangular region.", - required=["sheet", "base_cell", "row_count", "col_count"], - optional=[], - constraints=["row_count > 0", "col_count > 0", "or use range shorthand alias"], - example={ - "op": "draw_grid_border", - "sheet": "Sheet1", - "base_cell": "A1", - "row_count": 5, - "col_count": 4, - }, - aliases={"range": "base_cell + row_count + col_count"}, - ), - "set_bold": PatchOpSchema( - op="set_bold", - description="Apply bold font to one cell or one range.", - required=["sheet"], - optional=["cell", "range", "bold"], - constraints=["exactly one of cell or range"], - example={"op": "set_bold", "sheet": "Sheet1", "range": "A1:D1", "bold": True}, - ), - "set_font_size": PatchOpSchema( - op="set_font_size", - description="Apply font size to one cell or one range.", - required=["sheet", "font_size"], - optional=["cell", "range"], - constraints=["exactly one of cell or range", "font_size > 0"], - example={ - "op": "set_font_size", - "sheet": "Sheet1", - "cell": "A1", - "font_size": 14, - }, - ), - "set_font_color": PatchOpSchema( - op="set_font_color", - description="Apply font color to one cell or one range.", - required=["sheet", "color"], - optional=["cell", "range"], - constraints=[ - "exactly one of cell or range", - "hex color (#RRGGBB or #AARRGGBB)", - ], - example={ - "op": "set_font_color", - "sheet": "Sheet1", - "range": "A1:D1", - "color": "#1F4E79", - }, - ), - "set_fill_color": PatchOpSchema( - op="set_fill_color", - description="Apply fill color to one cell or one range.", - required=["sheet", "fill_color"], - optional=["cell", "range"], - constraints=[ - "exactly one of cell or range", - "hex color (#RRGGBB or #AARRGGBB)", - ], - example={ - "op": "set_fill_color", - "sheet": "Sheet1", - "range": "A1:D1", - "fill_color": "#D9E1F2", - }, - aliases={"color": "fill_color"}, - ), - "set_dimensions": PatchOpSchema( - op="set_dimensions", - description="Set row height and/or column width.", - required=["sheet"], - optional=["rows", "columns", "row_height", "column_width"], - constraints=[ - "at least one of row_height or column_width", - "row_height > 0, column_width > 0", - ], - example={ - "op": "set_dimensions", - "sheet": "Sheet1", - "rows": [1, 2], - "row_height": 22, - "columns": ["A", "B"], - "column_width": 18, - }, - aliases={ - "row": "rows", - "col": "columns", - "height": "row_height", - "width": "column_width", - }, - ), - "auto_fit_columns": PatchOpSchema( - op="auto_fit_columns", - description="Auto-fit column widths with optional width bounds.", - required=["sheet"], - optional=["columns", "min_width", "max_width"], - constraints=[ - "columns optional (uses used columns when omitted)", - "min_width > 0, max_width > 0 when provided", - "min_width <= max_width when both are provided", - ], - example={ - "op": "auto_fit_columns", - "sheet": "Sheet1", - "columns": ["A", 2], - "min_width": 8, - "max_width": 40, - }, - ), - "merge_cells": PatchOpSchema( - op="merge_cells", - description="Merge one rectangular range.", - required=["sheet", "range"], - optional=[], - constraints=["range must be rectangular"], - example={"op": "merge_cells", "sheet": "Sheet1", "range": "A1:C1"}, - ), - "unmerge_cells": PatchOpSchema( - op="unmerge_cells", - description="Unmerge merged cells intersecting range.", - required=["sheet", "range"], - optional=[], - constraints=["range must be rectangular"], - example={"op": "unmerge_cells", "sheet": "Sheet1", "range": "A1:C1"}, - ), - "set_alignment": PatchOpSchema( - op="set_alignment", - description="Set alignment flags to one cell or one range.", - required=["sheet"], - optional=["cell", "range", "horizontal_align", "vertical_align", "wrap_text"], - constraints=[ - "exactly one of cell or range", - "specify at least one alignment field", - ], - example={ - "op": "set_alignment", - "sheet": "Sheet1", - "range": "A1:D1", - "horizontal_align": "center", - "vertical_align": "center", - "wrap_text": True, - }, - aliases={"horizontal": "horizontal_align", "vertical": "vertical_align"}, - ), - "set_style": PatchOpSchema( - op="set_style", - description="Apply multiple style attributes in one op.", - required=["sheet"], - optional=[ - "cell", - "range", - "bold", - "font_size", - "color", - "fill_color", - "horizontal_align", - "vertical_align", - "wrap_text", - ], - constraints=[ - "exactly one of cell or range", - "specify at least one style field", - "font_size > 0", - "target cell count <= style limit", - ], - example={ - "op": "set_style", - "sheet": "Sheet1", - "range": "A1:D1", - "bold": True, - "color": "#FFFFFF", - "fill_color": "#1F3864", - "horizontal_align": "center", - }, - ), - "apply_table_style": PatchOpSchema( - op="apply_table_style", - description="Create table and apply Excel table style.", - required=["sheet", "range", "style"], - optional=["table_name"], - constraints=[ - "range must include header row", - "table name must be unique when provided", - "range must not intersect existing table", - ], - example={ - "op": "apply_table_style", - "sheet": "Sheet1", - "range": "A1:D11", - "style": "TableStyleMedium9", - "table_name": "SalesTable", - }, - ), - "create_chart": PatchOpSchema( - op="create_chart", - description="Create a new chart on a worksheet (COM backend only).", - required=["sheet", "chart_type", "data_range", "anchor_cell"], - optional=[ - "category_range", - "chart_name", - "width", - "height", - "titles_from_data", - "series_from_rows", - "chart_title", - "x_axis_title", - "y_axis_title", - ], - constraints=[ - "chart_type in {'line','column','bar','area','pie','doughnut','scatter','radar'}", - "data_range accepts A1 range string or list[str] for multi-series input", - "data_range/category_range allow sheet-qualified ranges like 'Sheet2!A1:B10'", - "width/height must be > 0 when provided", - "chart_name must be unique in sheet when provided", - "backend='openpyxl' is not supported", - ], - example={ - "op": "create_chart", - "sheet": "Sheet1", - "chart_type": "line", - "data_range": ["A2:A10", "C2:C10", "D2:D10"], - "category_range": "B2:B10", - "anchor_cell": "E2", - "chart_name": "SalesTrend", - "width": 360, - "height": 220, - "titles_from_data": True, - "series_from_rows": False, - "chart_title": "Sales trend", - "x_axis_title": "Month", - "y_axis_title": "Amount", - }, - ), - "restore_design_snapshot": PatchOpSchema( - op="restore_design_snapshot", - description="Internal inverse op to restore style snapshot.", - required=["sheet", "design_snapshot"], - optional=[], - constraints=["internal use for inverse operations"], - example={ - "op": "restore_design_snapshot", - "sheet": "Sheet1", - "design_snapshot": {"range": "A1:A1", "cells": []}, - }, - ), -} +from exstruct.edit.op_schema import ( + PatchOpSchema, + build_patch_tool_mini_schema, + get_patch_op_schema, + list_patch_op_schemas, + schema_with_sheet_resolution_rules, +) + +__all__ = [ + "PatchOpSchema", + "build_patch_tool_mini_schema", + "get_patch_op_schema", + "list_patch_op_schemas", + "schema_with_sheet_resolution_rules", +] diff --git a/src/exstruct/mcp/patch/chart_types.py b/src/exstruct/mcp/patch/chart_types.py index aeea4c34..e5910606 100644 --- a/src/exstruct/mcp/patch/chart_types.py +++ b/src/exstruct/mcp/patch/chart_types.py @@ -1,63 +1,21 @@ from __future__ import annotations -from typing import Final - -# Explicit ordered pairs of (chart_type, Excel COM ChartType ID). -# This is the single source of truth; both the ordered tuple and mapping are derived from it. -_CHART_TYPE_ENTRIES: Final[tuple[tuple[str, int], ...]] = ( - ("line", 4), - ("column", 51), - ("bar", 57), - ("area", 1), - ("pie", 5), - ("doughnut", -4120), - ("scatter", -4169), - ("radar", -4151), +from exstruct.edit.chart_types import ( + CHART_TYPE_ALIASES, + CHART_TYPE_TO_COM_ID, + SUPPORTED_CHART_TYPES, + SUPPORTED_CHART_TYPES_CSV, + SUPPORTED_CHART_TYPES_SET, + normalize_chart_type, + resolve_chart_type_id, ) -# Ordered tuple of canonical chart type names; ordering here determines error messages and docs. -SUPPORTED_CHART_TYPES: Final[tuple[str, ...]] = tuple(t for t, _ in _CHART_TYPE_ENTRIES) - -# Mapping from canonical chart type to Excel COM ChartType ID. -CHART_TYPE_TO_COM_ID: Final[dict[str, int]] = dict(_CHART_TYPE_ENTRIES) - -CHART_TYPE_ALIASES: Final[dict[str, str]] = { - "column_clustered": "column", - "bar_clustered": "bar", - "xy_scatter": "scatter", - "donut": "doughnut", -} - -SUPPORTED_CHART_TYPES_SET: Final[frozenset[str]] = frozenset(SUPPORTED_CHART_TYPES) -SUPPORTED_CHART_TYPES_CSV: Final[str] = ", ".join(SUPPORTED_CHART_TYPES) - - -def normalize_chart_type(chart_type: str) -> str | None: - """Normalize chart type input to a canonical key. - - Args: - chart_type: Raw chart type value from request payload. - - Returns: - Canonical chart type key when supported; otherwise ``None``. - """ - candidate = chart_type.strip().lower() - canonical = CHART_TYPE_ALIASES.get(candidate, candidate) - if canonical in SUPPORTED_CHART_TYPES_SET: - return canonical - return None - - -def resolve_chart_type_id(chart_type: str) -> int | None: - """Resolve canonical chart type key to Excel COM ChartType ID. - - Args: - chart_type: Raw or canonical chart type value. - - Returns: - Excel COM ChartType ID when supported; otherwise ``None``. - """ - normalized = normalize_chart_type(chart_type) - if normalized is None: - return None - return CHART_TYPE_TO_COM_ID[normalized] +__all__ = [ + "CHART_TYPE_ALIASES", + "CHART_TYPE_TO_COM_ID", + "SUPPORTED_CHART_TYPES", + "SUPPORTED_CHART_TYPES_CSV", + "SUPPORTED_CHART_TYPES_SET", + "normalize_chart_type", + "resolve_chart_type_id", +] diff --git a/src/exstruct/mcp/patch/normalize.py b/src/exstruct/mcp/patch/normalize.py index 41711ed9..ea8ec769 100644 --- a/src/exstruct/mcp/patch/normalize.py +++ b/src/exstruct/mcp/patch/normalize.py @@ -1,181 +1,25 @@ from __future__ import annotations -import json -from typing import Any, cast - -from exstruct.mcp.shared.a1 import parse_range_geometry - -from .specs import get_alias_map_for_op - - -def coerce_patch_ops(ops_data: list[dict[str, Any] | str]) -> list[dict[str, Any]]: - """Normalize patch operations payload for MCP clients.""" - normalized_ops: list[dict[str, Any]] = [] - for index, raw_op in enumerate(ops_data): - parsed_op = ( - dict(raw_op) - if isinstance(raw_op, dict) - else parse_patch_op_json(raw_op, index=index) - ) - normalized_ops.append(normalize_patch_op_aliases(parsed_op, index=index)) - return normalized_ops - - -def resolve_top_level_sheet_for_payload(data: object) -> object: - """Resolve top-level sheet default into operation dict payloads.""" - if not isinstance(data, dict): - return data - ops_raw = data.get("ops") - if not isinstance(ops_raw, list): - return data - top_level_sheet = normalize_top_level_sheet(data.get("sheet")) - resolved_ops: list[object] = [] - for index, op_raw in enumerate(ops_raw): - if not isinstance(op_raw, dict): - resolved_ops.append(op_raw) - continue - op_copy = normalize_patch_op_aliases(dict(op_raw), index=index) - op_name_raw = op_copy.get("op") - op_name = op_name_raw if isinstance(op_name_raw, str) else "" - op_sheet = op_copy.get("sheet") - if op_name == "add_sheet": - if op_copy.get("sheet") is None: - raise ValueError( - build_missing_sheet_message(index=index, op_name="add_sheet") - ) - resolved_ops.append(op_copy) - continue - if op_sheet is None: - if top_level_sheet is None: - raise ValueError( - build_missing_sheet_message(index=index, op_name=op_name) - ) - op_copy["sheet"] = top_level_sheet - resolved_ops.append(op_copy) - payload = dict(data) - payload["ops"] = resolved_ops - if top_level_sheet is not None: - payload["sheet"] = top_level_sheet - return payload - - -def normalize_patch_op_aliases( - op_data: dict[str, Any], *, index: int -) -> dict[str, Any]: - """Normalize MCP-friendly aliases to canonical patch operation fields.""" - normalized = dict(op_data) - op_name = normalized.get("op") - if not isinstance(op_name, str): - return normalized - alias_map = get_alias_map_for_op(op_name) - for alias, canonical in alias_map.items(): - alias_to_canonical_with_conflict_check( - normalized, - index=index, - alias=alias, - canonical=canonical, - op_name=op_name, - ) - normalize_draw_grid_border_range(normalized, index=index) - return normalized - - -def alias_to_canonical_with_conflict_check( - op_data: dict[str, Any], - *, - index: int, - alias: str, - canonical: str, - op_name: str, -) -> None: - """Map alias field to canonical field when operation type matches.""" - if op_data.get("op") != op_name or alias not in op_data: - return - alias_value = op_data[alias] - canonical_value = op_data.get(canonical) - if canonical in op_data: - if canonical_value != alias_value: - raise ValueError( - build_patch_op_error_message( - index, - f"conflicting fields: '{canonical}' and alias '{alias}'", - ) - ) - else: - op_data[canonical] = alias_value - del op_data[alias] - - -def normalize_draw_grid_border_range(op_data: dict[str, Any], *, index: int) -> None: - """Convert draw_grid_border range shorthand to base/size fields.""" - if op_data.get("op") != "draw_grid_border" or "range" not in op_data: - return - if "base_cell" in op_data or "row_count" in op_data or "col_count" in op_data: - raise ValueError( - build_patch_op_error_message( - index, - "draw_grid_border does not allow mixing 'range' with 'base_cell/row_count/col_count'", - ) - ) - range_ref = op_data.get("range") - if not isinstance(range_ref, str): - raise ValueError( - build_patch_op_error_message( - index, "draw_grid_border range must be a string A1 range" - ) - ) - try: - start, row_count, col_count = parse_range_geometry(range_ref) - except ValueError as exc: - raise ValueError( - build_patch_op_error_message( - index, "draw_grid_border range must be like 'A1:C3'" - ) - ) from exc - op_data["base_cell"] = start - op_data["row_count"] = row_count - op_data["col_count"] = col_count - del op_data["range"] - - -def parse_patch_op_json(raw_op: str, *, index: int) -> dict[str, Any]: - """Parse a JSON string patch operation into object form.""" - text = raw_op.strip() - if not text: - raise ValueError(build_patch_op_error_message(index, "empty string")) - try: - parsed = json.loads(text) - except json.JSONDecodeError as exc: - raise ValueError(build_patch_op_error_message(index, "invalid JSON")) from exc - if not isinstance(parsed, dict): - raise ValueError( - build_patch_op_error_message(index, "JSON value must be an object") - ) - return cast(dict[str, Any], parsed) - - -def normalize_top_level_sheet(value: object) -> str | None: - """Normalize optional top-level sheet text.""" - if not isinstance(value, str): - return None - candidate = value.strip() - return candidate or None - - -def build_missing_sheet_message(*, index: int, op_name: str) -> str: - """Build self-healing error for unresolved sheet selection.""" - target_op = op_name or "" - return ( - f"ops[{index}] ({target_op}) is missing sheet. " - "Set op.sheet, or set top-level sheet for non-add_sheet ops. " - "For add_sheet, op.sheet (or alias name) is required." - ) - - -def build_patch_op_error_message(index: int, reason: str) -> str: - """Build a consistent validation message for invalid patch ops.""" - example = '{"op":"set_value","sheet":"Sheet1","cell":"A1","value":"sample"}' - return ( - f"Invalid patch operation at ops[{index}]: {reason}. " - f"Use object form like {example}." - ) +from exstruct.edit.normalize import ( + alias_to_canonical_with_conflict_check, + build_missing_sheet_message, + build_patch_op_error_message, + coerce_patch_ops, + normalize_draw_grid_border_range, + normalize_patch_op_aliases, + normalize_top_level_sheet, + parse_patch_op_json, + resolve_top_level_sheet_for_payload, +) + +__all__ = [ + "alias_to_canonical_with_conflict_check", + "build_missing_sheet_message", + "build_patch_op_error_message", + "coerce_patch_ops", + "normalize_draw_grid_border_range", + "normalize_patch_op_aliases", + "normalize_top_level_sheet", + "parse_patch_op_json", + "resolve_top_level_sheet_for_payload", +] diff --git a/src/exstruct/mcp/patch/specs.py b/src/exstruct/mcp/patch/specs.py index af946dca..3e81543d 100644 --- a/src/exstruct/mcp/patch/specs.py +++ b/src/exstruct/mcp/patch/specs.py @@ -1,58 +1,5 @@ from __future__ import annotations -from typing import Final, cast +from exstruct.edit.specs import PATCH_OP_SPECS, PatchOpSpec, get_alias_map_for_op -from pydantic import BaseModel, Field - -from .types import PatchOpType - - -class PatchOpSpec(BaseModel): - """Specification metadata used by patch-op normalization.""" - - op: PatchOpType - aliases: dict[str, str] = Field(default_factory=dict) - - -PATCH_OP_SPECS: Final[dict[PatchOpType, PatchOpSpec]] = { - "set_value": PatchOpSpec(op="set_value"), - "set_formula": PatchOpSpec(op="set_formula"), - "add_sheet": PatchOpSpec(op="add_sheet", aliases={"name": "sheet"}), - "set_range_values": PatchOpSpec(op="set_range_values"), - "fill_formula": PatchOpSpec(op="fill_formula"), - "set_value_if": PatchOpSpec(op="set_value_if"), - "set_formula_if": PatchOpSpec(op="set_formula_if"), - "draw_grid_border": PatchOpSpec(op="draw_grid_border"), - "set_bold": PatchOpSpec(op="set_bold"), - "set_font_size": PatchOpSpec(op="set_font_size"), - "set_font_color": PatchOpSpec(op="set_font_color"), - "set_fill_color": PatchOpSpec(op="set_fill_color", aliases={"color": "fill_color"}), - "set_dimensions": PatchOpSpec( - op="set_dimensions", - aliases={ - "row": "rows", - "col": "columns", - "height": "row_height", - "width": "column_width", - }, - ), - "auto_fit_columns": PatchOpSpec(op="auto_fit_columns"), - "merge_cells": PatchOpSpec(op="merge_cells"), - "unmerge_cells": PatchOpSpec(op="unmerge_cells"), - "set_alignment": PatchOpSpec( - op="set_alignment", - aliases={"horizontal": "horizontal_align", "vertical": "vertical_align"}, - ), - "set_style": PatchOpSpec(op="set_style"), - "apply_table_style": PatchOpSpec(op="apply_table_style"), - "create_chart": PatchOpSpec(op="create_chart"), - "restore_design_snapshot": PatchOpSpec(op="restore_design_snapshot"), -} - - -def get_alias_map_for_op(op_name: str) -> dict[str, str]: - """Return alias mapping for one operation name.""" - if op_name not in PATCH_OP_SPECS: - return {} - spec = PATCH_OP_SPECS[cast(PatchOpType, op_name)] - return dict(spec.aliases) +__all__ = ["PATCH_OP_SPECS", "PatchOpSpec", "get_alias_map_for_op"] diff --git a/src/exstruct/mcp/patch/types.py b/src/exstruct/mcp/patch/types.py index 1175e318..d93f45e7 100644 --- a/src/exstruct/mcp/patch/types.py +++ b/src/exstruct/mcp/patch/types.py @@ -1,53 +1,27 @@ from __future__ import annotations -from typing import Literal +from exstruct.edit.types import ( + FormulaIssueCode, + FormulaIssueLevel, + HorizontalAlignType, + OnConflictPolicy, + PatchBackend, + PatchEngine, + PatchOpType, + PatchStatus, + PatchValueKind, + VerticalAlignType, +) -PatchOpType = Literal[ - "set_value", - "set_formula", - "add_sheet", - "set_range_values", - "fill_formula", - "set_value_if", - "set_formula_if", - "draw_grid_border", - "set_bold", - "set_font_size", - "set_font_color", - "set_fill_color", - "set_dimensions", - "auto_fit_columns", - "merge_cells", - "unmerge_cells", - "set_alignment", - "set_style", - "apply_table_style", - "create_chart", - "restore_design_snapshot", +__all__ = [ + "FormulaIssueCode", + "FormulaIssueLevel", + "HorizontalAlignType", + "OnConflictPolicy", + "PatchBackend", + "PatchEngine", + "PatchOpType", + "PatchStatus", + "PatchValueKind", + "VerticalAlignType", ] -PatchStatus = Literal["applied", "skipped"] -PatchValueKind = Literal["value", "formula", "sheet", "style", "dimension", "chart"] -PatchBackend = Literal["auto", "com", "openpyxl"] -PatchEngine = Literal["com", "openpyxl"] -FormulaIssueLevel = Literal["warning", "error"] -FormulaIssueCode = Literal[ - "invalid_token", - "ref_error", - "name_error", - "div0_error", - "value_error", - "na_error", - "circular_ref_suspected", -] - -HorizontalAlignType = Literal[ - "general", - "left", - "center", - "right", - "fill", - "justify", - "centerContinuous", - "distributed", -] -VerticalAlignType = Literal["top", "center", "bottom", "justify", "distributed"] diff --git a/tasks/feature_spec.md b/tasks/feature_spec.md index 9c08529e..140f86d4 100644 --- a/tasks/feature_spec.md +++ b/tasks/feature_spec.md @@ -21,3 +21,76 @@ - `adr-reconciler` audit scope: `ADR-0001`〜`ADR-0005`, `dev-docs/specs/excel-extraction.md`, `dev-docs/testing/test-requirements.md`, `docs/api.md`, `docs/cli.md`, `docs/mcp.md`, `AGENTS.md` - `adr-reconciler` findings: なし - next action: `no-action` + +## 2026-03-15 issue #99 phase 1 public edit API + +### Goal + +- Excel editing を `exstruct.edit` から利用できる first-class Python API として公開する。 +- `PatchOp` / `PatchRequest` / `MakeRequest` / `PatchResult` と既存 op 契約を維持したまま、MCP 固有の path policy を public API から外す。 +- MCP は互換レイヤとして維持し、tool I/O と host safety policy を担当し続ける。 + +### Public contract + +- Primary public import path: `exstruct.edit` +- Public entry points: + - `patch_workbook(request: PatchRequest) -> PatchResult` + - `make_workbook(request: MakeRequest) -> PatchResult` +- Public helpers: + - `coerce_patch_ops` + - `resolve_top_level_sheet_for_payload` + - `list_patch_op_schemas` + - `get_patch_op_schema` +- Preserved Phase 1 contract: + - existing op names remain unchanged + - existing warning/error payload shapes remain unchanged + - existing MCP compatibility imports remain valid + +### Boundary + +- `PathPolicy`, artifact mirroring, MCP tool payloads, server defaults, and thread offloading remain MCP-owned behavior. +- Phase 1 intentionally reuses the existing `exstruct.mcp.patch.*` execution pipeline under the hood to reduce backend regression risk while promoting the public API. + +### Permanent references + +- ADR: `dev-docs/adr/ADR-0006-public-edit-api-and-host-boundary.md` +- Internal specs: + - `dev-docs/specs/editing-api.md` + - `dev-docs/specs/data-model.md` + - `dev-docs/architecture/overview.md` +- Public docs: + - `docs/api.md` + - `docs/mcp.md` + +## 2026-03-15 pr #102 review follow-up + +### Goal + +- PR #102 の妥当な review 指摘だけを取り込み、Phase 1 の公開契約を変えずに不整合と正規化漏れを修正する。 +- `coerce_patch_ops` / `resolve_top_level_sheet_for_payload` の JSON-string op 対応を、dict op と同じ indexed error shape で安定化する。 +- ADR-0006 の status metadata を `accepted` に統一し、PR metadata warning は最小範囲で解消する。 + +### Accepted findings + +- `coerce_patch_ops([None])` が `AttributeError` を漏らし、indexed validation error にならない。 +- `resolve_top_level_sheet_for_payload` が JSON-string op を未解決のまま返し、top-level `sheet` fallback を適用できない。 +- `ADR-0006` の本文と index artifacts の status が不一致。 + +### Chosen constraints + +- public API signature と import path は変更しない。 +- invalid op 型の失敗は `ValueError(build_patch_op_error_message(...))` に統一する。 +- docstring warning 対応は、この PR で新規追加した `src/exstruct/edit/*.py` の不足 module docstring 補完までに限定する。 +- PR 本文は `.github/pull_request_template.md` の見出し構造に合わせるが、Acceptance Criteria は issue #99 phase 1 用に書き換える。 + +## 2026-03-15 pr #102 docs review follow-up + +### Goal + +- PR #102 の docs review 指摘のうち、import path 表記の誤りと architecture tree の欠落だけを最小差分で修正する。 +- 永続文書の説明を現行実装と一致させ、Phase 1 の API / runtime 契約自体は変更しない。 + +### Accepted findings + +- `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` が抜けている。 diff --git a/tasks/todo.md b/tasks/todo.md index 443bb358..1799a047 100644 --- a/tasks/todo.md +++ b/tasks/todo.md @@ -19,3 +19,66 @@ - `adr-reconciler` findings: なし - Verification: `rg -n "^## " tasks/feature_spec.md tasks/todo.md` - Verification: `git diff --check -- tasks/feature_spec.md tasks/todo.md` + +## 2026-03-15 issue #99 phase 1 public edit API + +### Planning + +- [x] issue #99 の Phase 1 境界を確認し、public API / MCP host boundary を整理する +- [x] `exstruct.edit` の公開面を定義する +- [x] op schema / normalize / type metadata の public import path を追加する +- [x] 既存 MCP compatibility path を維持する +- [x] ADR / internal spec / public docs を更新する +- [x] `uv run task precommit-run` を完走する + +### Review + +- `src/exstruct/edit/` を追加し、`patch_workbook` / `make_workbook` と patch 契約の公開 import path を導入した。 +- `exstruct.edit` は `PathPolicy` を要求せず、既存の patch request/result 契約をそのまま利用する。 +- `src/exstruct/mcp/patch/types.py`, `chart_types.py`, `specs.py`, `normalize.py`, `src/exstruct/mcp/op_schema.py` は `exstruct.edit` の契約モジュールを参照する互換 path に更新した。 +- 恒久文書として `dev-docs/specs/editing-api.md` と `dev-docs/adr/ADR-0006-public-edit-api-and-host-boundary.md` を追加し、ADR index artifacts も同期した。 +- Verification: + - `uv run pytest tests/edit/test_api.py` + - `uv run pytest tests/mcp/patch/test_normalize.py -q` + - `uv run pytest tests/mcp/patch/test_service.py tests/mcp/test_patch_runner.py tests/mcp/test_make_runner.py -q` + - `uv run task precommit-run` + +## 2026-03-15 pr #102 review follow-up + +### Planning + +- [x] `normalize.py` の non-dict/non-str op handling を indexed `ValueError` に統一する +- [x] top-level `sheet` fallback が JSON-string op にも適用されるようにする +- [x] ADR-0006 の status を本文と index artifacts で同期する +- [x] review regression tests を追加する +- [x] PR 本文を template structure に合わせて更新する +- [x] targeted pytest と `uv run task precommit-run` を実行する + +### Review + +- `src/exstruct/edit/normalize.py` に raw op coercion helper を追加し、unsupported 型を indexed `ValueError` で拒否するようにした。 +- `resolve_top_level_sheet_for_payload` は JSON-string op も dict 化してから alias 正規化と top-level `sheet` fallback を適用するようにした。 +- ADR-0006 の status は `README.md`, `index.yaml`, `decision-map.md` を `accepted` に揃えた。 +- review regression tests を `tests/mcp/patch/test_normalize.py`, `tests/mcp/test_tool_models.py`, `tests/mcp/test_server.py` に追加した。 +- docstring warning 対応として、新規 `src/exstruct/edit/*.py` の不足 module docstring を補った。 +- PR 本文は `.github/pull_request_template.md` の見出し構造に合わせて更新した。 +- Verification: + - `uv run pytest tests/mcp/patch/test_normalize.py tests/mcp/test_tool_models.py tests/mcp/test_server.py -q` + - `uv run task precommit-run` + +## 2026-03-15 pr #102 docs review follow-up + +### Planning + +- [x] unresolved review thread の内容を確認し、実文書との差分だけを直す +- [x] `data-model.md` の import path 表記を Python module path に揃える +- [x] `architecture/overview.md` の `edit/` tree を実ファイル構成に合わせる +- [x] 文書差分を確認し、必要なら PR thread を resolve する + +### Review + +- `dev-docs/specs/data-model.md` の “actual locations” は filesystem path ではなく Python import path を示す表現に修正した。 +- `dev-docs/architecture/overview.md` の `edit/` tree に `chart_types.py` と `errors.py` を追加した。 +- Verification: + - `git diff --check` + - `uv run task precommit-run` diff --git a/tests/edit/test_api.py b/tests/edit/test_api.py new file mode 100644 index 00000000..140d3975 --- /dev/null +++ b/tests/edit/test_api.py @@ -0,0 +1,79 @@ +from __future__ import annotations + +from pathlib import Path + +from openpyxl import Workbook, load_workbook + +from exstruct.edit import ( + MakeRequest, + PatchOp, + PatchRequest, + get_patch_op_schema, + make_workbook, + patch_workbook, +) +from exstruct.mcp.patch_runner import PatchRequest as McpPatchRequest + + +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 test_patch_workbook_edits_without_path_policy(tmp_path: Path) -> None: + source = tmp_path / "book.xlsx" + _create_workbook(source) + + result = patch_workbook( + PatchRequest( + xlsx_path=source, + ops=[PatchOp(op="set_value", sheet="Sheet1", cell="A1", value="new")], + backend="openpyxl", + ) + ) + + assert result.error is None + patched = Path(result.out_path) + assert patched.exists() + workbook = load_workbook(patched) + try: + assert workbook["Sheet1"]["A1"].value == "new" + finally: + workbook.close() + + +def test_make_workbook_creates_file_without_path_policy(tmp_path: Path) -> None: + target = tmp_path / "new_book.xlsx" + + result = make_workbook( + MakeRequest( + out_path=target, + ops=[ + PatchOp(op="add_sheet", sheet="Data"), + PatchOp(op="set_value", sheet="Data", cell="A1", value="ok"), + ], + backend="openpyxl", + ) + ) + + assert result.error is None + workbook = load_workbook(target) + try: + assert workbook["Data"]["A1"].value == "ok" + finally: + workbook.close() + + +def test_edit_request_import_path_matches_mcp_compatibility_path() -> None: + assert PatchRequest is McpPatchRequest + + +def test_edit_op_schema_is_public() -> None: + schema = get_patch_op_schema("create_chart") + assert schema is not None + assert schema.op == "create_chart" diff --git a/tests/mcp/patch/test_normalize.py b/tests/mcp/patch/test_normalize.py index 99b379e2..714b6c63 100644 --- a/tests/mcp/patch/test_normalize.py +++ b/tests/mcp/patch/test_normalize.py @@ -55,6 +55,14 @@ def test_coerce_patch_ops_normalizes_draw_grid_border_range() -> None: assert "range" not in result[0] +def test_coerce_patch_ops_rejects_non_object_non_string_payload() -> None: + with pytest.raises( + ValueError, + match=r"Invalid patch operation at ops\[0\]: patch op must be an object or JSON string, got NoneType: None", + ): + coerce_patch_ops([None]) + + def test_resolve_top_level_sheet_for_payload() -> None: payload = { "sheet": "Sheet1", @@ -70,8 +78,34 @@ def test_resolve_top_level_sheet_for_payload() -> None: assert ops[1]["sheet"] == "Data" +def test_resolve_top_level_sheet_for_payload_normalizes_json_string_ops() -> None: + payload = { + "sheet": "Sheet1", + "ops": ['{"op":"set_value","cell":"A1","value":"x"}'], + } + resolved = resolve_top_level_sheet_for_payload(payload) + assert isinstance(resolved, dict) + ops = resolved["ops"] + assert ops[0] == { + "op": "set_value", + "sheet": "Sheet1", + "cell": "A1", + "value": "x", + } + + def test_resolve_top_level_sheet_for_payload_rejects_missing_sheet() -> None: with pytest.raises(ValueError, match="missing sheet"): resolve_top_level_sheet_for_payload( {"ops": [{"op": "set_value", "cell": "A1", "value": "x"}]} ) + + +def test_resolve_top_level_sheet_for_payload_rejects_non_object_non_string_ops() -> ( + None +): + with pytest.raises( + ValueError, + match=r"Invalid patch operation at ops\[0\]: patch op must be an object or JSON string, got NoneType: None", + ): + resolve_top_level_sheet_for_payload({"sheet": "Sheet1", "ops": [None]}) diff --git a/tests/mcp/test_server.py b/tests/mcp/test_server.py index f57b7101..2b4326f8 100644 --- a/tests/mcp/test_server.py +++ b/tests/mcp/test_server.py @@ -1023,6 +1023,74 @@ async def fake_run_sync(func: Callable[[], object]) -> object: assert make_call[0].ops[1].value == "x" +def test_register_tools_applies_top_level_sheet_fallback_to_json_string_patch_ops( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + app = DummyApp() + policy = PathPolicy(root=tmp_path) + calls: dict[str, tuple[object, ...]] = {} + + def fake_run_extract_tool( + payload: ExtractToolInput, + *, + policy: PathPolicy, + on_conflict: OnConflictPolicy, + ) -> ExtractToolOutput: + return ExtractToolOutput(out_path="out.json") + + def fake_run_read_json_chunk_tool( + payload: ReadJsonChunkToolInput, + *, + policy: PathPolicy, + ) -> ReadJsonChunkToolOutput: + return ReadJsonChunkToolOutput(chunk="{}") + + def fake_run_validate_input_tool( + payload: ValidateInputToolInput, + *, + policy: PathPolicy, + ) -> ValidateInputToolOutput: + return ValidateInputToolOutput(is_readable=True) + + def fake_run_patch_tool( + payload: PatchToolInput, + *, + policy: PathPolicy, + on_conflict: OnConflictPolicy, + ) -> PatchToolOutput: + calls["patch"] = (payload, policy, on_conflict) + return PatchToolOutput(out_path="out.xlsx", patch_diff=[], engine="openpyxl") + + async def fake_run_sync(func: Callable[[], object]) -> object: + return func() + + monkeypatch.setattr(server, "run_extract_tool", fake_run_extract_tool) + monkeypatch.setattr( + server, "run_read_json_chunk_tool", fake_run_read_json_chunk_tool + ) + monkeypatch.setattr(server, "run_validate_input_tool", fake_run_validate_input_tool) + monkeypatch.setattr(server, "run_patch_tool", fake_run_patch_tool) + monkeypatch.setattr(anyio.to_thread, "run_sync", fake_run_sync) + + server._register_tools(app, policy, default_on_conflict="overwrite") + patch_tool = cast(Callable[..., Awaitable[object]], app.tools["exstruct_patch"]) + anyio.run( + _call_async, + patch_tool, + { + "xlsx_path": "in.xlsx", + "sheet": "Sheet1", + "ops": ['{"op":"set_value","cell":"A1","value":"x"}'], + }, + ) + patch_call = cast( + tuple[PatchToolInput, PathPolicy, OnConflictPolicy], calls["patch"] + ) + assert patch_call[0].sheet == "Sheet1" + assert patch_call[0].ops[0].sheet == "Sheet1" + assert patch_call[0].ops[0].cell == "A1" + + def test_register_tools_accepts_merge_and_alignment_json_strings( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: diff --git a/tests/mcp/test_tool_models.py b/tests/mcp/test_tool_models.py index c5b69478..7c6785f9 100644 --- a/tests/mcp/test_tool_models.py +++ b/tests/mcp/test_tool_models.py @@ -128,6 +128,16 @@ def test_patch_tool_input_applies_top_level_sheet_fallback() -> None: assert payload.ops[0].sheet == "Sheet1" +def test_patch_tool_input_applies_top_level_sheet_to_json_string_op() -> None: + payload = PatchToolInput( + xlsx_path="input.xlsx", + sheet="Sheet1", + ops=['{"op":"set_value","cell":"A1","value":"x"}'], + ) + assert payload.ops[0].sheet == "Sheet1" + assert payload.ops[0].cell == "A1" + + def test_patch_tool_input_prioritizes_op_sheet_over_top_level() -> None: payload = PatchToolInput( xlsx_path="input.xlsx", @@ -171,6 +181,16 @@ def test_make_tool_input_applies_top_level_sheet_fallback() -> None: assert payload.ops[0].sheet == "Sheet1" +def test_make_tool_input_applies_top_level_sheet_to_json_string_op() -> None: + payload = MakeToolInput( + out_path="output.xlsx", + sheet="Sheet1", + ops=['{"op":"set_value","cell":"A1","value":"x"}'], + ) + assert payload.ops[0].sheet == "Sheet1" + assert payload.ops[0].cell == "A1" + + def test_make_tool_input_accepts_add_sheet_name_alias() -> None: payload = MakeToolInput( out_path="output.xlsx",