PIGS-811: Add outputTarget to JSON-returning tools for inline results#81
Merged
Conversation
JSON-producing tools wrote output to disk and returned only a filename, which Claude's container cannot read back, breaking multi-step flows where a JSON step is intermediate. Add a model-selected outputTarget param (inline | file | both, default inline) via a shared jsonResult helper, so extracted data is returned inline by default and can drive follow-up steps without filesystem access. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the Node.js MCP tool layer to support inline JSON results (optionally alongside file output) via a shared outputTarget parameter, enabling multi-step/chained flows without relying on filesystem reads.
Changes:
- Add
outputTarget(inline|file|both, defaultinline) to multiple extraction/PII tools and return JSON inline underdatawhen requested. - Introduce
node-version/src/tools/jsonOutput.ts(jsonResult) to centralize inline/file/both shaping for JSON-returning tools. - Update Vitest expectations for extraction + PII tools to cover default inline behavior and explicit file output.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
node-version/src/tools/jsonOutput.ts |
New helper to shape JSON tool results for inline/file/both outputs. |
node-version/src/tools/extractions.ts |
Apply outputTarget + jsonResult to JSON-returning extraction tools; update text extraction output behavior. |
node-version/src/tools/pii.ts |
Apply outputTarget + jsonResult to extract_pii; expand redact tool parameter description. |
node-version/src/models.ts |
Add outputTargetSchema shared input parameter. |
node-version/tests/extractions.test.ts |
Update/expand tests for inline-by-default JSON extraction behavior and file-only cases. |
node-version/tests/pii.test.ts |
Update/expand tests for inline-by-default PII extraction and file-only cases. |
node-version/src/tools/transformations.ts |
Adjust watermark boundingBox schema typing (tuple → length-4 array). |
node-version/src/handlers/platformHandler.ts |
Loosen WatermarkParams.boundingBox type to `number[] |
CLAUDE.md |
Document the new outputTarget convention and shared helper usage. |
pyproject.toml |
Add Python local-dev dependencies (despite file being marked legacy/archived). |
uv.lock |
Lockfile updates for newly added Python local-dev dependencies (and related package updates). |
|
|
||
| export interface WatermarkParams { | ||
| readonly boundingBox?: [number, number, number, number] | null; | ||
| readonly boundingBox?: number[] | null; |
Contributor
There was a problem hiding this comment.
is this broader definition better?
Contributor
Author
There was a problem hiding this comment.
I have a local script which uses an agent to test the tool before I create a PR, when it was trying to load the tuple, it's not compatible for some reason.
- Correct jsonOutput.ts helper path in CLAUDE.md to node-version/src/... - Reword extract_pii and search_text_in_pdf descriptions to reflect the inline-by-default outputTarget behaviour instead of "Returns a JSON file" - Generalise outputTargetSchema description to cover JSON or text output 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Use the shared `data` key for extract_pdf_text's inline payload instead of `text`, matching every other outputTarget tool so results chain generically. Update tests accordingly. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
outputTargetparam (inline|file|both, defaultinline) to every JSON-returning tool so extracted data is returned directly in the tool result instead of only being written to disk.src/tools/jsonOutput.ts(jsonResulthelper) to centralize inline/file/both result shaping.get_pdf_metadata,extract_pdf_forms,extract_pdf_tables,extract_pdf_text,extract_invoice_data,search_text_in_pdf, andextract_pii.Why
Multi-step flows with a data step were broken: JSON-returning endpoints wrote results to the filesystem and returned only a filename, but Claude can't read the file back — so it couldn't chain steps (e.g. extract forms from several PDFs into one Excel, or PII-extract then redact a filtered subset). Defaulting to
inlinereturns parsed data underdataso follow-up steps can use it, and keeps transient flows from littering the working folder.file/bothstill write to disk and returnoutputFilename. Binary outputs (Excel/PDF) remain file-only and are orthogonal tooutputTarget.Test plan
task n:check(format + types + lint) passestask n:testpasses (161 passed, 1 skipped)🤖 Generated with Claude Code