docx/pdf/xlsx: six defects in the document skills (verified at 34040c9)

All six defects below reproduce at 34040c9. I found them by inspection in August against f6656c1 and re-verified each one against the source today. All six files are byte-identical between those two commits, so every line number below is equally valid against either.

They fall into three groups:

• 1, 2 and 5 report success, or exit 0, without checking that what they claim to have done actually happened.
• 6 is the inverse — a false positive that blocks a correct workbook.
• 3 and 4 are documented commands that cannot run as written. These fail loudly; they are cheap to fix and they sit at step 1 of their workflows.

1. skills/docx/scripts/accept_changes.py:76 — a LibreOffice timeout is reported as success

    except subprocess.TimeoutExpired:
        return (
            None,
            f"Successfully accepted all tracked changes: {input_file} -> {output_file}",
        )

soffice operates in place on a copy of the input, and the macro stores and closes the document itself. A timeout therefore leaves the output in an unknown state — possibly untouched, possibly partly processed — and the caller is told it succeeded. __main__ tests if "Error" in message, so the process also exits 0.

Fix: return an error on timeout, and check the postcondition — that the output document no longer carries revision marks — before claiming success.

2. skills/docx/scripts/comment.py:180 — comments are added to a document that cannot show them

def _ensure_comment_relationships(unpacked_dir: Path) -> None:
    rels_path = unpacked_dir / "word" / "_rels" / "document.xml.rels"
    if not rels_path.exists():
        return

word/_rels/document.xml.rels is optional in a valid minimal DOCX. When it is absent this returns before adding any relationship, but add_comment carries on regardless: comments.xml is written, the comment XML is appended, and the run reports Wrote <file>. The comment part exists with nothing referencing it, so Word cannot resolve it. (The message's existing caveat is about adding markers to word/document.xml, which is a different requirement and does not cover this.)

Fix: create the relationships part when it is missing, rather than skipping.

3. skills/pdf/forms.md:4 — the first mandatory command cannot run

 `python scripts/check_fillable_fields <file.pdf>`

The file is check_fillable_fields.py, and skills/pdf/scripts/ contains no extensionless entry point. This is step 1 of a workflow the same file opens by saying "You MUST complete these steps in order", so the documented path fails at its first command.

Fix: add the .py.

4. skills/pdf/scripts/convert_pdf_to_images.py:20 — writes into a directory it never creates

        image_path = os.path.join(output_dir, f"page_{i+1}.png")
        image.save(image_path)

There is no os.makedirs. forms.md invokes the converter three times — lines 54, 178 and 289 — each with an output directory that nothing in the documented flow creates, so the conversion raises FileNotFoundError whenever that directory does not already exist.

Fix: os.makedirs(output_dir, exist_ok=True) before the loop.

5. skills/pdf/scripts/check_bounding_boxes.py:46-47 — a height in pixels is compared against a font size in points

                font_size = ri.field["entry_text"].get("font_size", 14)
                entry_height = ri.rect[3] - ri.rect[1]
                if entry_height < font_size:

Under Approach B the boxes are image pixel coordinates — forms.md B.4 signals this with image_width/image_height — while font_size is in points, so the two sides of this comparison are in different units. (Under Approach A, where the boxes are PDF points, the comparison is consistent.)

The scale is not a fixed constant, which is what makes this awkward to fix. convert_pdf_to_images.py renders at 200 DPI but then downscales any image whose longest side exceeds max_dim=1000 (lines 9-18). A US Letter page becomes 1700x2200 and is then resized to 772x1000 — an effective 90.9 DPI, about 1.26 pixels per point, not the 2.78 the nominal 200 DPI would suggest. Pages of different sizes in one document land at different effective scales.

So converting with a hardcoded DPI would keep the check wrong. The checker needs the page's height in points alongside its height in pixels, and in Approach B fields.json supplies only the pixel dimensions. Options: carry both dimensions in fields.json and scale by image_height / pdf_height; pass the source PDF; or stop downscaling so a known DPI holds.

Separately, and independent of the units: __main__ prints the messages and falls off the end, so the script exits 0 after printing FAILURE and a caller testing the exit status sees a pass.

Fix: derive the threshold from the actual per-page pixel-to-point ratio rather than a constant, and exit nonzero when any message is a FAILURE.

6. skills/xlsx/scripts/recalc.py:237 — error tokens are substring-matched against prose

                        for err in excel_errors:
                            if err in cell.value:

cell.value here is any string cell, so a cell reading Use #N/A when unavailable is counted as an error and status comes back as errors_found on a clean workbook. skills/xlsx/SKILL.md makes that consequential: recalculation is "mandatory whenever the file contains formulas", and "Never ship while recalc.py reports errors_found." A correct workbook that happens to mention an error token in prose cannot pass that gate.

Fix: match error-typed cells — openpyxl exposes these as data_type == "e" — or compare against the exact cached error value, rather than a substring of arbitrary text.

---

I checked the open issues first: #1120 (an AF_UNIX guard in office/soffice.py, reached from recalc.py) and #1464 (defined-name case sensitivity in recalc.py) are different defects, and none of the six above is already filed.

These four document skills are source-available rather than open source, so this is a report and not a pull request. Each fix is a few lines; if you would like patches, say so and I will send them.