Skip to content

docs: correct mermaid-py behavior in comments (remote API, not local Node.js) - #110

Merged
anhnh2002 merged 2 commits into
FSoft-AI4Code:mainfrom
kiboook:fix/mermaid-py-docs-comment
Sep 16, 2026
Merged

anhnh2002 merged 2 commits into
FSoft-AI4Code:mainfrom
kiboook:fix/mermaid-py-docs-comment

Conversation

@kiboook

@kiboook kiboook commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Related to #109 — posting this alongside that issue in case my reading is correct, easy to close either without merging if I'm wrong about something.

mermaid-py (>=0.8.0, the version pinned in pyproject.toml) validates diagrams by sending them to a remote rendering service (https://mermaid.ink by default, overridable via mermaid-py's own MERMAID_INK_SERVER env var) — not by spawning a local Node.js subprocess. I checked this in both mermaid-py==0.8.0 and the current 0.8.4: Mermaid.__init__ calls _make_request_to_mermaid(), which does a plain requests.get(). No subprocess/child_process/Popen/os.system anywhere in the package.

This PR only corrects the wording in two comments to match:

  • codewiki/src/be/utils.py: the _MERMAID_PY_BROKEN comment
  • pyproject.toml: the nodejs build-requires comment

No functional change. I left the nodejs build-requires entry itself alone since I couldn't confirm whether it's needed for something unrelated to mermaid-py — happy to remove it too if you confirm it isn't.

…Node.js)

mermaid-py (>=0.8.0, the version this project pins) validates diagrams by
sending them to a remote rendering service (https://mermaid.ink by default,
overridable via mermaid-py's own MERMAID_INK_SERVER env var), not by
spawning a local Node.js subprocess. Verified by inspecting the mermaid-py
0.8.0 and 0.8.4 source directly (mermaid/__main__.py uses requests.get()
against MERMAID_INK_SERVER, no subprocess/child_process anywhere).

This corrects two comments that describe the old (inaccurate, or possibly
just always-wrong) assumption:
- codewiki/src/be/utils.py: the _MERMAID_PY_BROKEN comment
- pyproject.toml: the nodejs build-requires comment

No functional change. The nodejs build-requires entry itself is left as-is
since I couldn't confirm whether it's needed for something else in the
project; flagging it for maintainers to reconsider.
@kiboook

kiboook commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

I noticed the CI / Lint check is failing here, so I dug into it a bit before assuming it was caused by this PR.

It looks unrelated to this diff. The 16 ruff violations reported all come from lines this PR doesn't touch (import ordering, deprecated typing.List/Tuple, an unused traceback import, a few blind except Exception: blocks, blocking open() inside an async function, etc.) — pre-existing patterns in utils.py.

Reproduced locally: with ruff==0.15.12 (what the pinned ruff>=0.1.0 in pyproject.toml would resolve to on an older run) only 1 violation shows up, but with ruff==0.16.7 (what the Lint job actually installed, per the run log) the same 16 appear — including against the unmodified main copy of the file, before my change. So it seems like a newer ruff release expanded its default rule set, and this file's existing code just wasn't linted against those rules before.

Not sure if pinning a ruff version (or adding a [tool.ruff.lint] select) is something you'd want, so I'll leave that to you — just wanted to flag that this failure isn't from the two-line comment fix in this PR.

…ils.py

- pyproject.toml: state why Node.js/npm is still required — PythonMonkey
  (via mermaid-parser-py) shells out to npm at install time. mermaid-py
  itself validates via the remote mermaid.ink API.
- pyproject.toml: pin [tool.ruff.lint] select to ruff's pre-0.16 defaults
  so the unpinned `pip install ruff` in CI stops flagging pre-existing
  code whenever a file is touched.
- utils.py: note that validation sends diagram content to a third-party
  service; fix a stale "broken Node.js setup" comment; drop the unused
  traceback import; apply `ruff format` so the CI format check passes.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@anhnh2002

anhnh2002 commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Thanks for this. Your reading of mermaid-py is correct (verified against 0.8.4: Mermaid.__init__requests.get() against MERMAID_INK_SERVER, no subprocess anywhere).

I pushed a small follow-up commit on top of yours (8be46ef) rather than round-tripping via review comments:

  • Answering your open question on nodejs: it is still needed, just not for mermaid-py. PythonMonkey (pulled in via mermaid-parser-py, the primary validation path in utils.py) shells out to npm at install time through its pminit helper to populate its JS dependencies. I rewrote the pyproject.toml comment to say that instead of the "pending confirmation" hedge, and left the build-requires entry in place.
  • Added a clause to the utils.py comment noting that validation sends diagram content to the third-party service (worth knowing when deciding on MERMAID_VALIDATE=0), and fixed a second stale "broken Node.js setup" comment further down in the same file.

Will merge once CI is green. Closes #109.

@anhnh2002
anhnh2002 merged commit a089266 into FSoft-AI4Code:main Sep 16, 2026
2 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.

2 participants