Complete issue #99 Phase 3: rewire MCP to the public edit core

dev-docs/architecture/overview.md

@@ -31,9 +31,17 @@ exstruct/
   render/
   edit/
     __init__.py
+    a1.py
     api.py
     chart_types.py
+    engine/
+      __init__.py
+      openpyxl_engine.py
+      xlwings_engine.py
     errors.py
+    internal.py
+    output_path.py
+    runtime.py
     service.py
     models.py
     normalize.py
@@ -95,23 +103,25 @@ CLI entry point
 
 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
+- `api.py` → public patch/make entry points for Python callers
+- `service.py` → canonical patch/make orchestration used by both Python API and MCP
+- `models.py` → canonical edit request/result models
+- `runtime.py` → canonical backend selection, fallback, and policy-free path/runtime helpers
+- `internal.py` → edit-owned low-level patch implementation and structured patch errors
+- `output_path.py` → edit-owned output/conflict helpers reusable by host shims
+- `engine/*` → canonical backend execution boundaries
+- `a1.py` → A1 helpers owned by the edit core
 - `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
+- `edit/` does not import `mcp/`; MCP is allowed to depend on `edit`, not vice versa
 
 ### mcp/patch (Patch Implementation)
 
 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)
-- `patch/service.py` → orchestration of `run_patch` / `run_make`
-- `patch/runtime.py` → runtime utilities for path/backend selection
-- `patch/engine/openpyxl_engine.py` → openpyxl execution boundary
-- `patch/engine/xlwings_engine.py` → xlwings (COM) execution boundary
-- `patch/ops/openpyxl_ops.py` → op application entry point for openpyxl
-- `patch/ops/xlwings_ops.py` → op application entry point for xlwings
+- `patch_runner.py` → compatibility facade for maintaining existing import paths and syncing host overrides
+- `patch/internal.py` → compatibility facade re-exporting edit-owned internal implementation
+- `patch/service.py` / `patch/runtime.py` / `patch/engine/*` → compatibility shims around `exstruct.edit`
+- `patch/ops/openpyxl_ops.py` / `patch/ops/xlwings_ops.py` → legacy op entry points kept for compatibility
 - `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
 

dev-docs/specs/data-model.md

@@ -266,10 +266,11 @@ The model group used by workbook editing remains importable from both
 The actual locations are as follows.
 
 - Primary public import path: `exstruct.edit` / `exstruct.edit.models`
-- Current backing implementation module: `exstruct.mcp.patch.models`
+- Current backing implementation module: `exstruct.edit.models`
+- Current non-public execution helpers: `exstruct.edit.internal`, `exstruct.edit.runtime`, `exstruct.edit.engine.*`
 - 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`
+- MCP integration layer import path: `exstruct.mcp.patch.service` (shim)
 
 Primary models:
 

dev-docs/specs/editing-api.md

@@ -52,11 +52,21 @@ remain owned by MCP / agent hosts:
 
 ## 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:
+- `exstruct.edit` is now both the canonical public import path and the
+  canonical editing core implementation boundary.
+- `src/exstruct/edit/**` does not import `exstruct.mcp.*`; MCP depends downward
+  on the edit core, not the other way around.
+- Canonical edit-core modules:
+  - `exstruct.edit.models`
+  - `exstruct.edit.internal`
+  - `exstruct.edit.runtime`
+  - `exstruct.edit.engine.*`
+  - `exstruct.edit.service`
+- `exstruct.mcp.patch_runner` and `exstruct.mcp.patch.*` remain compatibility
+  and host-integration facades around that edit core.
+- `PathPolicy` path canonicalization is resolved in the MCP integration layer
+  before requests are handed to `exstruct.edit.service`.
+- Contract metadata moved to `exstruct.edit`:
   - patch op types
   - chart type metadata
   - patch op alias/spec metadata

docs/mcp.md

@@ -295,23 +295,20 @@ Example:
 }
 ```
 
-### Internal implementation note
+### 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.
+Workbook editing also has a public Python import path under `exstruct.edit`.
+MCP remains the host/integration layer and keeps `PathPolicy`, transport, and
+tool payload concerns outside that public API.
 
-The patch implementation is layered to keep compatibility while enabling refactoring:
+For MCP users, the stable surfaces are:
 
 - `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 the Python API and host policy to
-evolve independently.
+- `exstruct.mcp.patch_runner`: compatibility facade for existing import paths
+- MCP server / tool entrypoints: host-owned path policy, transport, and artifact behavior
+
+Internal module layering is documented in
+`dev-docs/architecture/overview.md` and `dev-docs/specs/editing-api.md`.
 
 ## Edit flow (patch)
 

src/exstruct/edit/a1.py

@@ -0,0 +1,94 @@
+"""A1 helpers used by the public workbook editing core."""
+
+from __future__ import annotations
+
+import re
+
+_A1_PATTERN = re.compile(r"^[A-Za-z]{1,3}[1-9][0-9]*$")
+_A1_RANGE_PATTERN = re.compile(r"^[A-Za-z]{1,3}[1-9][0-9]*:[A-Za-z]{1,3}[1-9][0-9]*$")
+_COLUMN_LABEL_PATTERN = re.compile(r"^[A-Za-z]{1,3}$")
+
+
+def split_a1(value: str) -> tuple[str, int]:
+    """Split A1 notation into normalized (column_label, row_index)."""
+    if not _A1_PATTERN.match(value):
+        raise ValueError(f"Invalid cell reference: {value}")
+    idx = 0
+    for index, char in enumerate(value):
+        if char.isdigit():
+            idx = index
+            break
+    column = value[:idx].upper()
+    row = int(value[idx:])
+    return column, row
+
+
+def column_label_to_index(label: str) -> int:
+    """Convert Excel-style column label (A/AA) to 1-based index."""
+    normalized = label.strip().upper()
+    if not _COLUMN_LABEL_PATTERN.match(normalized):
+        raise ValueError(f"Invalid column label: {label}")
+    index = 0
+    for char in normalized:
+        index = index * 26 + (ord(char) - ord("A") + 1)
+    return index
+
+
+def column_index_to_label(index: int) -> str:
+    """Convert 1-based column index to Excel-style column label."""
+    if index < 1:
+        raise ValueError("Column index must be positive.")
+    chunks: list[str] = []
+    current = index
+    while current > 0:
+        current -= 1
+        chunks.append(chr(ord("A") + (current % 26)))
+        current //= 26
+    return "".join(reversed(chunks))
+
+
+def normalize_range(value: str) -> str:
+    """Validate and normalize an A1 range string."""
+    candidate = value.strip()
+    if not _A1_RANGE_PATTERN.match(candidate):
+        raise ValueError(f"Invalid range reference: {value}")
+    start, end = candidate.split(":", maxsplit=1)
+    return f"{start.upper()}:{end.upper()}"
+
+
+def range_cell_count(range_ref: str) -> int:
+    """Return the number of cells represented by an A1 range."""
+    start, end = normalize_range(range_ref).split(":", maxsplit=1)
+    start_col, start_row = split_a1(start)
+    end_col, end_row = split_a1(end)
+    min_col = min(column_label_to_index(start_col), column_label_to_index(end_col))
+    max_col = max(column_label_to_index(start_col), column_label_to_index(end_col))
+    min_row = min(start_row, end_row)
+    max_row = max(start_row, end_row)
+    return (max_col - min_col + 1) * (max_row - min_row + 1)
+
+
+def parse_range_geometry(range_ref: str) -> tuple[str, int, int]:
+    """Parse A1 range and return top-left cell + (rows, cols)."""
+    start_ref, end_ref = normalize_range(range_ref).split(":", maxsplit=1)
+    start_col, start_row = split_a1(start_ref)
+    end_col, end_row = split_a1(end_ref)
+    min_col = min(column_label_to_index(start_col), column_label_to_index(end_col))
+    max_col = max(column_label_to_index(start_col), column_label_to_index(end_col))
+    min_row = min(start_row, end_row)
+    max_row = max(start_row, end_row)
+    return (
+        f"{column_index_to_label(min_col)}{min_row}",
+        max_row - min_row + 1,
+        max_col - min_col + 1,
+    )
+
+
+__all__ = [
+    "column_index_to_label",
+    "column_label_to_index",
+    "normalize_range",
+    "parse_range_geometry",
+    "range_cell_count",
+    "split_a1",
+]

src/exstruct/edit/api.py

@@ -3,7 +3,20 @@
 from __future__ import annotations
 
 from .models import MakeRequest, PatchRequest, PatchResult
-from .service import make_workbook, patch_workbook
+from .service import make_workbook as _make_workbook, patch_workbook as _patch_workbook
+
+
+def patch_workbook(request: PatchRequest) -> PatchResult:
+    """Edit an existing workbook without MCP path policy enforcement."""
+
+    return _patch_workbook(request)
+
+
+def make_workbook(request: MakeRequest) -> PatchResult:
+    """Create a new workbook and apply initial patch operations."""
+
+    return _make_workbook(request)
+
 
 __all__ = [
     "make_workbook",

src/exstruct/edit/engine/__init__.py

@@ -0,0 +1,8 @@
+"""Canonical engine entry points for the public workbook editing core."""
+
+from __future__ import annotations
+
+from .openpyxl_engine import apply_openpyxl_engine
+from .xlwings_engine import apply_xlwings_engine
+
+__all__ = ["apply_openpyxl_engine", "apply_xlwings_engine"]

src/exstruct/edit/engine/openpyxl_engine.py

@@ -0,0 +1,63 @@
+"""Canonical openpyxl engine boundary for workbook editing."""
+
+from __future__ import annotations
+
+from collections.abc import Sequence
+from pathlib import Path
+from typing import Any, TypeVar, cast
+
+from pydantic import BaseModel, ValidationError
+
+from exstruct.edit import internal as _internal
+from exstruct.edit.models import (
+    FormulaIssue,
+    OpenpyxlEngineResult,
+    PatchDiffItem,
+    PatchOp,
+    PatchRequest,
+)
+
+TModel = TypeVar("TModel", bound=BaseModel)
+
+
+def apply_openpyxl_engine(
+    request: PatchRequest,
+    input_path: Path,
+    output_path: Path,
+) -> OpenpyxlEngineResult:
+    """Apply patch operations using the edit-owned openpyxl implementation."""
+    diff, inverse_ops, formula_issues, op_warnings = _internal._apply_ops_openpyxl(
+        cast(Any, request),
+        input_path,
+        output_path,
+    )
+    return OpenpyxlEngineResult(
+        patch_diff=_coerce_model_list(diff, PatchDiffItem),
+        inverse_ops=_coerce_model_list(inverse_ops, PatchOp),
+        formula_issues=_coerce_model_list(formula_issues, FormulaIssue),
+        op_warnings=list(op_warnings),
+    )
+
+
+def _coerce_model_list(
+    items: Sequence[object], model_cls: type[TModel]
+) -> list[TModel]:
+    """Normalize model-like payloads into canonical Pydantic models."""
+    coerced: list[TModel] = []
+    for item in items:
+        try:
+            source: object
+            if isinstance(item, model_cls):
+                coerced.append(item)
+                continue
+            if isinstance(item, BaseModel):
+                source = item.model_dump(mode="python")
+            else:
+                source = item
+            coerced.append(model_cls.model_validate(source))
+        except ValidationError:
+            continue
+    return coerced
+
+
+__all__ = ["apply_openpyxl_engine"]

src/exstruct/edit/engine/xlwings_engine.py

@@ -0,0 +1,28 @@
+"""Canonical xlwings engine boundary for workbook editing."""
+
+from __future__ import annotations
+
+from pathlib import Path
+from typing import Any, cast
+
+from exstruct.edit import internal as _internal
+from exstruct.edit.models import PatchOp
+
+
+def apply_xlwings_engine(
+    input_path: Path,
+    output_path: Path,
+    ops: list[PatchOp],
+    auto_formula: bool,
+) -> list[object]:
+    """Apply patch operations using the edit-owned xlwings implementation."""
+    diff = _internal._apply_ops_xlwings(
+        input_path,
+        output_path,
+        cast(list[Any], ops),
+        auto_formula,
+    )
+    return list(diff)
+
+
+__all__ = ["apply_xlwings_engine"]

src/exstruct/edit/errors.py

@@ -2,6 +2,6 @@
 
 from __future__ import annotations
 
-from exstruct.mcp.patch.ops.common import PatchOpError
+from .internal import PatchOpError
 
 __all__ = ["PatchOpError"]