Skip to content

Name drawio beside SVG where local files are said to be read as text - #111

Merged
caufieldjh merged 4 commits into
mainfrom
docs-drawio-file-source
Oct 13, 2026
Merged

caufieldjh merged 4 commits into
mainfrom
docs-drawio-file-source

Conversation

@caufieldjh

Copy link
Copy Markdown
Contributor

The FileSource docstring and the Limitations list in docs/how-to/use-local-files-and-urls.md still named only SVG as the exception to reading local files as raw text. .drawio and .drawio.xml files are also read through their extractor (DrawioExtractor). This brings both up to date. No behaviour change.

Last loose end for #104.

just test: 1841 passed, mypy and ruff clean.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @caufieldjh's task in 26s —— View job


PR Review: Name drawio beside SVG where local files are said to be read as text

  • Read the diff and the surrounding code
  • Check the docstring and docs claims against the code
  • Look for other places that still list only SVG
  • Post review

Summary

The changes are correct. I checked each claim against the code:

  • resolve_format maps both .drawio and .drawio.xml to "drawio" (etl/acquire.py:29-30).
  • FIGURE_CONTENT_TYPES includes drawio (matching.py:15), so FileSource.fetch sends these files through _fetch_figure.
  • The DrawioExtractor cross-reference in the docstring points to a real module (etl/extract/drawio.py).
  • A figure with no extracted text becomes unavailable (file.py:129), and that holds for drawio as well as SVG. So the reworded sentence "A figure with no rendered text … is unavailable" is accurate.

There is no behaviour change, so there are no security or performance concerns.

Suggestions (minor, optional)

  1. The module docstring is still out of date. src/linkml_reference_validator/etl/sources/file.py:3 still says:

    Reads content from local files (markdown, text, HTML, SVG).

    This is the same staleness the PR fixes in the class docstring, three dozen lines further down. Suggest (markdown, text, HTML, SVG, drawio).
    Fix this →

  2. Same issue in the extractor package docstring, outside this PR's diff. src/linkml_reference_validator/etl/extract/__init__.py:1 reads """Content extractors (PDF, HTML, XML, SVG). and doesn't mention drawio. You could fix it here, since the PR is meant to close out Extract rendered text from SVG (and drawio) so diagrams can be used as references #104.

  3. The Limitations bullet only names .drawio. In docs/how-to/use-local-files-and-urls.md:274, .drawio.xml is not listed. The "see above" link covers it, since the drawio section at line 208 lists both extensions, so this is fine to leave. Naming both (.drawio/.drawio.xml) would match the class docstring.

Test coverage

This PR only changes docs and docstrings, so it needs no new tests. The _fetch_figure doctest still uses SVG only. A drawio doctest would also document the second path, but drawio extraction is presumably already covered by the #104 tests, so this isn't required.

LGTM once you've decided on item 1.

caufieldjh and others added 3 commits October 13, 2026 13:09
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @caufieldjh's task in 22s —— View job


PR Review: Name drawio beside SVG where local files are said to be read as text

  • Read the updated diff (4 commits)
  • Check the earlier suggestions against the new commits
  • Check the docstring and docs claims against the code
  • Post review

Summary

LGTM. The new commits cover all three suggestions from my last review:

  1. The module docstring in etl/sources/file.py:3 now says (markdown, text, HTML, SVG, drawio).
  2. The extractor package docstring in etl/extract/__init__.py:1 now says (PDF, HTML, XML, SVG, drawio).
  3. The Limitations bullet in docs/how-to/use-local-files-and-urls.md:274 now names both .drawio and .drawio.xml.

Accuracy check

  • The class docstring says "a figure with no rendered text, or whose rendered text cannot be determined, is unavailable". That holds for drawio as well as SVG. DrawioExtractor raises Unrenderable in the same situations: bad base64/deflate, inflation past the size limit, a missing mxGraphModel, and placeholder-computed labels (etl/extract/drawio.py:178-239). An empty result maps to unavailable in _fetch_figure (file.py:129).
  • The "see above" link in Limitations still works. The drawio section ("Diagrams as References (drawio)", line 206) is above it and lists both extensions.
  • No stale mentions remain. I found no other prose in src/ or docs/ that still names SVG as the only figure type.

Code quality / bugs / performance / security

This PR only changes docs and docstrings, with no behaviour change. I found no bugs, performance issues or security issues.

Test coverage

No new tests are needed. One optional idea: the _fetch_figure doctest (file.py:109-118) only covers SVG, though its summary line now says "an SVG or drawio figure". A small uncompressed .drawio example would document the second path too. The #104 tests already cover drawio extraction, so this is not a blocker.

@caufieldjh
caufieldjh merged commit d205ae0 into main Oct 13, 2026
5 checks passed
@caufieldjh
caufieldjh deleted the docs-drawio-file-source branch October 13, 2026 17:27
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.

1 participant