Skip to content

fix(processing): honor a tool's recommended output extension over the… - #1901

Merged
giswqs merged 2 commits into
opengeos:mainfrom
Ahmedseko:fix/excel-to-table-output-extension
Aug 15, 2026
Merged

fix(processing): honor a tool's recommended output extension over the…#1901
giswqs merged 2 commits into
opengeos:mainfrom
Ahmedseko:fix/excel-to-table-output-extension

Conversation

@Ahmedseko

@Ahmedseko Ahmedseko commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

… table default

excel_to_table's output param is data_kind "table", so leaving the output path blank made the WASM runner default it to .csv. Its writer is the generic vector-format dispatch (same as GeoParquet/GPKG tools), which has no CSV driver, so the tool failed its own default with "unsupported output path" -- readable input (21542 rows x 10 columns from a legacy .xls), unusable default output. Same failure shape as #1074.

The param's own description already names the extension that works ("GeoParquet .parquet recommended"); outputTextFormatHint now honors an explicit " recommended" in the description before falling back to the generic csv/html/json/table heuristics.

Summary by CodeRabbit

  • New Features

    • Output file formats can now follow an explicitly recommended extension in the output description.
    • Supports any bare file extension, beyond previously recognized formats.
    • Recommended extensions are normalized to lowercase.
  • Bug Fixes

    • Recommended extensions now take priority over the default CSV format for table outputs.
    • Explicitly selected output paths continue to be honored.
    • Decimal values in descriptions are no longer mistaken for file extensions.

… table default

excel_to_table's output param is data_kind "table", so leaving the
output path blank made the WASM runner default it to .csv. Its writer
is the generic vector-format dispatch (same as GeoParquet/GPKG tools),
which has no CSV driver, so the tool failed its own default with
"unsupported output path" -- readable input (21542 rows x 10 columns
from a legacy .xls), unusable default output. Same failure shape as
opengeos#1074.

The param's own description already names the extension that works
("GeoParquet .parquet recommended"); outputTextFormatHint now honors
an explicit "<ext> recommended" in the description before falling
back to the generic csv/html/json/table heuristics.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 0634b853-0966-4802-9710-994e81fc6258

📥 Commits

Reviewing files that changed from the base of the PR and between 1c36d74 and 8802423.

📒 Files selected for processing (2)
  • packages/processing/src/wasm-client.ts
  • tests/wasm-tool-manifests.test.ts

📝 Walkthrough

Walkthrough

The processing client now recognizes an explicit recommended output extension before applying other format heuristics. Tests verify that the recommendation overrides the table default, explicit paths remain authoritative, and decimal values are ignored.

Changes

Output extension hints

Layer / File(s) Summary
Extension detection and validation
packages/processing/src/wasm-client.ts, tests/wasm-tool-manifests.test.ts
outputTextFormatHint accepts bare recommended extensions and prioritizes them. Tests cover GeoParquet recommendations, explicit path precedence, and decimal values in descriptions.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 88024

This localized change makes the tool honor its recommended output extension when no output path is provided. No actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: giswqs

Poem

A rabbit checks each output name,
A recommended hint leads the game.
Explicit paths remain in place,
Decimal prose leaves no trace.
Parquet hops ahead of CSV—
Clean extensions, happily!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: honoring a tool's recommended output extension over the default.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed: dependency version conflict. Check your lock file or package.json.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a Whitebox WASM “default output path” failure for table-producing tools (notably excel_to_table) by preferring the tool author’s explicitly recommended output extension when the user leaves the output path blank, instead of blindly defaulting data_kind: "table" outputs to .csv.

Changes:

  • Updated outputTextFormatHint to detect and honor a ".<ext> recommended" hint in an output parameter’s description before applying existing CSV/HTML/JSON/table heuristics.
  • Added a regression test asserting that a recommended extension (e.g. .parquet) overrides the table default while still letting an explicit user-chosen path win.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/wasm-tool-manifests.test.ts Adds coverage ensuring “.parquet recommended” in a table output’s description overrides the .csv default when the output path is blank.
packages/processing/src/wasm-client.ts Implements recommended-extension parsing in outputTextFormatHint, which feeds the WASM runner’s and dialog’s shared output extension logic.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@giswqs

giswqs commented Aug 15, 2026

Copy link
Copy Markdown
Member

/claude-review

@github-actions

github-actions Bot commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

🔍 Cloudflare PR preview

Item Value
Site https://73170e4a.geolibre-preview.pages.dev
Demo app https://73170e4a.geolibre-preview.pages.dev/demo/
Commit 8802423

Comment thread packages/processing/src/wasm-client.ts Outdated
@github-actions

Copy link
Copy Markdown
Contributor

Code review

Bugs

  • outputTextFormatHint's new /\.([a-z0-9]+)\s+recommended/i regex can false-positive on a decimal number immediately preceding "recommended" (e.g. "0.5 recommended"), extracting a bogus non-alphabetic "extension". Suggested a minimal fix requiring the capture to start with a letter. Confidence: low-medium (couldn't confirm against the live WASM manifest whether any current output-param description actually triggers this).

Security

  • None found.

Performance

  • None found — the added regex match is O(description length) and runs once per output-param render, negligible.

Quality

  • The fix is narrowly scoped, well-documented (docstring and inline comment explain the #1074-style failure mode being fixed), and the new test exercises the exact excel_to_table scenario plus confirms an explicit user-chosen path still takes priority. No concerns beyond the regex edge case above.

CLAUDE.md

  • No violations. This isn't one of the documented "mirror" constants that need re-checking on dependency bumps, and the change doesn't touch any of the areas called out in the repo's conventions.

@github-actions

github-actions Bot commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

🔍 GitHub Pages PR preview

Item Value
Site https://opengeos.org/pages-preview/GeoLibre/pr-1901/
Demo app https://opengeos.org/pages-preview/GeoLibre/pr-1901/demo/
Commit 8802423

Note

GitHub Pages built this preview successfully, but its serving edge returned HTTP 403 when checked. The links may still be propagating.

- Require the recommended-extension capture in `outputTextFormatHint` to
  start with a letter (`/\.([a-z][a-z0-9]*)\s+recommended/i`), so a decimal
  in a parameter's prose ("a tolerance of 0.5 recommended") can no longer be
  read as an extension and short-circuit the csv/html/json/table heuristics.
- Add a regression test covering that case.
@giswqs
giswqs merged commit c635d89 into opengeos:main Aug 15, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants