Skip to content

fix(skills): request markitdown extras in markdown-converter commands - #43

Merged
steipete merged 1 commit into
steipete:mainfrom
pereponkin:fix/markitdown-extras
Sep 30, 2026
Merged

steipete merged 1 commit into
steipete:mainfrom
pereponkin:fix/markitdown-extras

Conversation

@pereponkin

@pereponkin pereponkin commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

The documented PDF and Office commands installed only MarkItDown's base package, so conversions failed with missing optional dependencies. Request markitdown[all] in every invocation, including stdin examples, while retaining on-demand execution and quoting that works in POSIX shells, PowerShell, and Windows cmd.

Also document the existing charset override for text files whose encoding is detected incorrectly. Refs #41. Thanks @pereponkin for the report, fix, and Windows verification.

Validation: skill metadata validation; synthetic PDF, DOCX, XLSX, PPTX and PDF-stdin conversions; base-only PDF failure reproduced before the fix; full repository CI; independent review with no actionable findings.

@clawsweeper

clawsweeper Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 16, 2026
@clawsweeper

clawsweeper Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed September 29, 2026, 8:53 PM ET / September 30, 2026, 00:53 UTC (Revision 4).

ClawSweeper review

What this changes

The Markdown conversion skill now requests MarkItDown’s optional dependencies in its command examples and explains how to override incorrect text encoding detection.

Merge readiness

✅ Ready for maintainer review

The reported PDF and Office conversion failure remains in current main, and this PR is a focused candidate fix. The earlier changelog concern does not apply to the current head: the entry was added in a repository-owner commit after the contributor removed their entry.

Priority: P2
Reviewed head: dc7f3584bd7f30ce2d3b1948a14420447eb1ffce

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The patch is focused, its reported conversion checks cover the affected formats, and no actionable defect remains in the current head.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (terminal): The skill's changed owner is the uvx command documentation. The linked Windows report records a real PDF failure and successful conversion with the all extra, while the PR body reports after-fix conversions across PDF and Office formats, including stdin; no stored-data contract changes.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): The skill's changed owner is the uvx command documentation. The linked Windows report records a real PDF failure and successful conversion with the all extra, while the PR body reports after-fix conversions across PDF and Office formats, including stdin; no stored-data contract changes.
Evidence reviewed 6 items Current main still has the reported command: The main-branch skill invokes bare uvx markitdown for PDF and Office examples.
Introduced command change: The pinned PR delta changes the documented invocations to uvx --from "markitdown[all]" markitdown, including the stdin examples.
Reported real conversion: The linked issue reports a base-package PDF dependency failure and a successful conversion of the same file with MarkItDown's all extra on Windows 11; the PR body reports after-fix PDF, DOCX, XLSX, PPTX, and PDF-stdin conversions.
Findings None None.
Security None None.

How this fits together

The Markdown conversion skill gives agents commands for turning documents into Markdown. Its examples invoke MarkItDown through uvx, which selects the installed package dependencies before conversion runs.

flowchart LR
A[Document or stdin] --> B[Skill command]
B --> C[uvx selects MarkItDown extras]
C --> D[MarkItDown converts input]
D --> E[Markdown output]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

None.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #41
Summary: This PR is the candidate fix for the linked PDF and Office command failure report.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Technical review

Best possible solution:

Land the focused skill correction with its contributor credit, then resolve the linked failure report through the merged fix.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main still uses the bare package command, and the linked report records a missing optional-dependency failure for PDF with a successful conversion when the all extra is requested.

Is this the best way to solve the issue?

Yes. Requesting the existing package extra at the uvx entrypoint addresses the documented commands without adding an installation step or a new option.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 5008579e73be.

Labels

Label changes:

No label changes.

Label justifications:

  • P2: The documented commands fail for advertised PDF and Office formats, with a bounded skill-level correction.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The skill's changed owner is the uvx command documentation. The linked Windows report records a real PDF failure and successful conversion with the all extra, while the PR body reports after-fix conversions across PDF and Office formats, including stdin; no stored-data contract changes.
  • proof: sufficient: Contributor real behavior proof is sufficient. The skill's changed owner is the uvx command documentation. The linked Windows report records a real PDF failure and successful conversion with the all extra, while the PR body reports after-fix conversions across PDF and Office formats, including stdin; no stored-data contract changes.

Evidence

What I checked:

  • Current main still has the reported command: The main-branch skill invokes bare uvx markitdown for PDF and Office examples. (skills/markdown-converter/SKILL.md:14, 5008579e73be)
  • Introduced command change: The pinned PR delta changes the documented invocations to uvx --from "markitdown[all]" markitdown, including the stdin examples. (skills/markdown-converter/SKILL.md:14, dc7f3584bd7f)
  • Reported real conversion: The linked issue reports a base-package PDF dependency failure and a successful conversion of the same file with MarkItDown's all extra on Windows 11; the PR body reports after-fix PDF, DOCX, XLSX, PPTX, and PDF-stdin conversions.
  • Changelog policy and current authorship: AGENTS.MD reserves contributor-PR changelog additions for a maintainer or AI at landing. The prior contributor entry was removed, and the current entry arrived in the repository owner's new commit, as reflected in the supplied timeline and local commit record. (AGENTS.MD:87, 5008579e73be)
  • Feature history: Local file history identifies Peter Steinberger's earlier Markdown converter skill commit and subsequent description changes. Older blob retrieval failed, so this does not establish exact source-line introduction. (skills/markdown-converter/SKILL.md, bd47c356371b)
  • Release and main check: The current main blob still has the failing bare invocation, and no release tag contains the unmerged PR head; the supplied latest release is 0.12.0. (skills/markdown-converter/SKILL.md:14, 5008579e73be)

Likely related people:

  • Peter Steinberger: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (3 earlier review cycles)
  • reviewed 2026-09-16T12:33:26.608Z sha 50cffe6 :: needs changes before merge. :: [P3] Leave the changelog entry to the landing step
  • reviewed 2026-09-16T12:39:04.214Z sha 50cffe6 :: needs changes before merge. :: [P3] Leave the changelog entry to the landing step
  • reviewed 2026-09-16T13:15:14.387Z sha b2e82f2 :: needs maintainer review before merge. :: none

@pereponkin

Copy link
Copy Markdown
Contributor Author

Fixed — the CHANGELOG.md entry is removed in b2e82f2, leaving one file changed.

You were right and I was wrong on this: I read the general changelog rule earlier in AGENTS.MD and stopped there, missing line 86, which is the one that governs contributor PRs. Adding the entry was straightforwardly against policy.

The release-note material now lives in the PR body under a "Release note context" heading, per AGENTS.MD:85 — behaviour, surface, refs and contributor credit — so the landing step has what it needs without me touching the changelog.

Written by Claude (Anthropic) at the reporter's request.

Co-authored-by: pereponkin <pereponkin@gmail.com>
@steipete
steipete force-pushed the fix/markitdown-extras branch from b2e82f2 to dc7f358 Compare September 30, 2026 00:49
@steipete
steipete merged commit d15557c into steipete:main Sep 30, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants