refactor: unify PPTX import entry points — remove pptx_to_json from the MCP surface

[naohito2000]

Background

pptx_to_json and upload_file are two MCP entry points to the same conversion engine. shared/ingest.py:589 shows upload_file calling sdpm.converter.pptx_to_json internally — so upload_file is not a sibling of pptx_to_json, it is a layer on top of it.

They differ only in what they return and where they write:

	upload_file	pptx_to_json
Return value	{uploadId, fileName, slideCount, themeHints, guideInstruction} — a few hundred bytes	the full converted deck (every element of every slide)
Output location	session storage (.sessions/{sessionId}/uploads/{shortId}_{filename}/)	sibling dir of the input PPTX ({stem}/)
Follow-up path	import_attachment(uploadId, deck_id)	none — caller handles deck_dir itself
Guide steering	guideInstruction → read_guides(["import-pptx"])	none

Measured: for skill/references/examples/patterns.pptx (18 slides), the pptx_to_json return value is 293,365 chars (≈98k tokens), averaging 16.3 KB per slide. Breakdown by element type: group 65%, textbox 15%, shape 5%, freeform 2%, line 2%. (group dominates because it nests children recursively; table-heavy decks shift the peak to tables instead.)

Why this is debt

1. Two entry points to one operation, and the safe one is only signposted in prose. The pptx_to_json docstring already says callers who need an editable deck should prefer upload → import_attachment, but nothing caps the payload. A single call can consume a large share of a session's context budget.

2. upload_file already implements the right contract; pptx_to_json is the outlier. upload_file returns a handle for every input type and never inlines content:

   Input	Returns	Content inlined?
   PPTX (deck-structured)	uploadId + slideCount + themeHints + guideInstruction	no — deliberately omits even filePath
   PDF / DOCX / XLSX	uploadId + filePath (converted .md) + imagesDir	no
   Image	uploadId + filePath + colorAnalysis	no
   Text / CSV / JSON	uploadId + filePath	no

   The only inlined data are themeHints and colorAnalysis — small summaries needed to decide what to do next. The PPTX case even documents why filePath is withheld:

   # Deck-structure PPTX: filePath would only point at deck.json and miss
   # slides/, so the guide/themeHints fields below replace it.

3. The split is historical, not designed. upload_file was introduced with upload_tools.py in v0.2.0: Agent Separation, Parallel Slide Generation & Model Config Refactor #71 to add session storage for the Web UI (L4) — browser-uploaded files have no local absolute path, hence the uploadId indirection, and non-PPTX formats route through the same door. pptx_to_json predates that as a straight MCP passthrough of the engine function, mirroring the scripts/pptx_to_json.py CLI.

4. Name collision makes the safe path unreachable in practice. upload_file collides with at least two other MCP servers in a common setup (chrome-devtools: "Upload a file through a provided element"; slack: "Upload one or more files to Slack"). When another server occupies the always-loaded tool slot, upload_file resolves to that server even when the sdpm one is explicitly requested — leaving pptx_to_json as the only reachable route. This is how the 98k-token call above actually happened. The collision is environment-side and cannot be fixed from this repo; only a rename can.

   Same latent risk applies to other generically named sdpm tools: grid, run_python, search_assets.

L3 already models the target shape

Neither tool exists on mcp-server/ (L3), and that is not an omission — on Cloud, uploading happens outside MCP (the Web UI posts via the API; the agent receives an uploadId in an [Attached: ...] message, see the comment at mcp-server/server.py:57). What L3 exposes is only the downstream half:

L3 tool	Role
read_uploaded_file(upload_id, offset=0, limit=2000)	partial read of an uploaded file — paging is in the signature, with a default limit
import_attachment(source, deck_id, filename)	bring an uploadId or URL into the deck workspace

There is structurally no way to get a whole-deck payload back from an L3 tool. L2 has the same mechanism available (_format_cat_n with offset/limit in upload_tools.py) but does not make it part of the public contract.

Proposed direction

• Remove pptx_to_json from the MCP surface, keeping the engine function and the CLI. No in-repo consumer reads its return value:

  • mcp-local/{server,server_acp,server_with_instruction}.py — registration only
  • skill/sdpm/diff/ — invokes scripts/pptx_to_json.py as a subprocess and reads from disk
  • tests/test_converter_robustness.py — calls the engine function directly

  Import then has one entry point: upload_file → import_attachment.

• Rename the surviving tool to avoid the collision. Existing sdpm tools use verb+object (init_presentation, import_attachment, generate_pptx), so something like stage_upload fits; a sdpm_-prefixed name would be inconsistent with the rest of the surface.

• Treat the L3 split as the reference design for L2: convert-and-stage returns a handle, and reading content is a separate tool with explicit paging. If pptx_to_json is kept instead, make its return value a handle to match ({deck_dir, slide_count, fonts, themeHints}) and point callers at run_python / read_json for per-slide reads. Verified safe: no in-repo consumer breaks.

• Adopt "tools return handles, not payloads" as an explicit rule for new tools, alongside a naming rule that avoids generic verbs likely to collide with other MCP servers.

Non-goals

• No change to mcp-server/ (L3) — its division of labour is the reference, not the problem.
• No change to the converter output format itself. Payload-size reductions inside the JSON (table cell style inheritance, adjacent-run merging, externalizing _pathLstXml) are schema work tracked separately.
• No change to diff_pptx. Its table blowup has the same root cause (cells are full-style dicts, and _diff_value in skill/sdpm/diff/__init__.py truncates strings but not dicts/lists), so it should be revisited after the schema change rather than patched now.

Notes

• L2 passes a fixed session_id="mcp-local" (mcp-local/server.py:135), so all local uploads accumulate under .sessions/mcp-local/uploads/. Whether anything prunes that directory is unverified.
• upload_tools.py already has _format_cat_n (offset/limit line-numbered output), i.e. partial reads are an established pattern in this module.