Skip to content

fix(parsers): MermaidParser no longer silently drops edge lines, plus metadata pass - #3

Merged
thorwhalen merged 3 commits into
masterfrom
wads/fix-mermaid-parser-and-metadata
Jul 30, 2026
Merged

fix(parsers): MermaidParser no longer silently drops edge lines, plus metadata pass#3
thorwhalen merged 3 commits into
masterfrom
wads/fix-mermaid-parser-and-metadata

Conversation

@thorwhalen

Copy link
Copy Markdown
Member

The bug

MermaidParser discarded any edge line it could not match, without a word. A
live gpt-4o-mini call produced this:

flowchart TD
    start([Start]) --> check_user{Is user authenticated?}
    check_user --|Yes|--> show_dashboard([Show Dashboard])
    check_user --|No|--> show_login([Show Login])
    show_dashboard --> end([End])
    show_login --> end([End])

check_user --|Yes|--> show_dashboard is invalid Mermaid — the valid form is
check_user -->|Yes| show_dashboard. The parser dropped both of those lines,
and the first line fared no better: an edge line carrying inline node
definitions failed the edge patterns, fell through to the node parser, which
matched whichever shape came first in its pattern list and threw the rest away.

Net result — the parser returned this, reporting nothing wrong:

nodes DECISION nodes edges
what the diagram says 5 1 (check_user) 5
what the parser returned 4 0 2

check_user{Is user authenticated?} was gone. show_dashboard and
show_login were typed START. A diagram that should contain a decision
contained none, and nothing anywhere said so. This is how model drift becomes a
silently wrong diagram rather than an error.

This is not hypothetical drift: it is exactly why
test_llm_converter_real_api_with_decision fails today
(assert len(decision_nodes) >= 1assert 0 >= 1) on any machine with
OPENAI_API_KEY exported. The A[Start] --> B[End] idiom — the most common
Mermaid form there is — was equally affected; the pre-existing
test_llm_converter_code_block_cleanup used it and passed anyway, because it
only asserted validate(), which an under-populated diagram satisfies.

The fix

ij/parsers/mermaid.py:

  • Edge endpoints are resolved individually, so inline node shapes on an
    edge line are kept and typed correctly.
  • Non-canonical link spellings are normalized to the canonical
    ARROW|label| form: the invalid -- |label| --> drift, plus the valid
    Mermaid alternatives a -- label --> b and a -. label .-> b which were
    also being dropped.
  • Chained edge lines (a --> b --> c) yield one edge per link. Previously
    only the first link survived.
  • Nothing is dropped mutely. An uninterpretable line is recorded in
    MermaidParser.unparsed_lines and raised as a MermaidParseWarning; the new
    keyword-only strict=True turns it into the ValueError that parse's
    docstring has always promised but never raised.
  • Valid Mermaid must not trip that warning, so comments (%%),
    subgraph/end/direction/style/classDef/click/linkStyle
    directives, bare node ids, and the legacy graph keyword are now recognized
    rather than falling through.
  • Node shapes are a single table driving both the node and endpoint patterns,
    instead of two hand-maintained pattern lists.

ij/converters/llm_converter.py: convert, refine and
convert_with_examples all route through a new _parse_mermaid, which refuses
output holding no valid diagram (with the raw model output in the message)
instead of returning an empty one. MermaidParseWarning is exported from ij
so a caller can escalate it with warnings.simplefilter("error", ...).

Confirmed against the live API: with this fix,
test_llm_converter_real_api_with_decision passes against real gpt-4o-mini —
the root cause was the parser, not the model.

Tests

before after
gate (priv test-dependents ij) 123 passed, 1 failed 131 passed, 2 skipped
CI dry run (wads doctest flags) 123 passed, 1 failed 131 passed, 2 skipped

Eight new hermetic tests (no network). Six in tests/test_parsers.py pin:
inline node definitions surviving; the --|label|--> normalization (asserted
under simplefilter("error"), so a dropped line fails the test); the
-- label --> / -. label .-> forms; chained edges; the warning and
unparsed_lines on genuinely unreadable input; strict=True raising; and a
guard that a valid diagram full of comments, subgraphs and style directives
produces no warning at all. Two in tests/test_llm_converter.py pin the
end-to-end drift case (the captured model output above, asserting the DECISION
node survives — the hermetic stand-in for the live test) and the refusal of
unusable output.

Live-API tests are now opt-in

Both real-API tests were gated on mere presence of OPENAI_API_KEY, so any
developer or agent with a key exported silently made non-hermetic,
network-dependent, cost-incurring calls. They now require an explicit
IJ_RUN_REAL_API_TESTS. CI behaviour is unchanged — it sets neither variable,
so both were and remain skipped there.

Metadata pass

  • license: the [project.license] table with the non-SPDX value "mit"
    becomes the inline SPDX string license = "MIT". Verified in the built
    wheel: License-Expression: MIT, Metadata-Version: 2.4. No
    License :: OSI Approved :: trove classifier is added alongside it — PEP 639
    rejects that combination.
  • authors: was [], so PyPI showed no author.
  • classifiers: none were declared. The 3.10/3.12 pair mirrors the CI matrix.
  • .editorconfig: added, byte-identical to the fleet template.
  • ci.yml: dropped two stale migration comments (no setup.cfg exists;
    PYPI_PASSWORD is provisioned and Publish has succeeded). No behavioural
    change.
  • Repo metadata set out of band: homepage → https://i2mint.github.io/ij/, plus
    the seven topics mirroring the pyproject keywords. Description was already
    correct and was left alone.

Deliberately left out of scope

  • A validate-then-retry loop in LLMConverter. The parser fix plus a hard
    failure on unusable output covers the observed drift; a retry loop is a
    bigger design question (retry budget, prompt repair, cost) and belongs in its
    own PR.
  • Mermaid open links (a --- b, a -.- b) are still unsupported — but they
    now warn instead of vanishing.
  • ij.__version__ is "0.2.0" while pyproject and PyPI are at 0.1.5.
    Pre-existing drift, untouched.
  • Package doctests never run. testpaths = ["tests"] means the wads CI
    invocation collects only tests/, so ij/'s doctests are not executed
    anywhere. Two are broken or non-hermetic:
    LLMConverter.refine's raises NameError: name 'converter' is not defined,
    and ij/export/image.py's writes a diagram.png into the repo root. Changing
    testpaths would change what the gate collects, so it is left for a separate
    decision.

https://claude.ai/code/session_01VipiLaG4xy7WctqY9w2475

MermaidParser discarded any edge line it could not match, without a word.
Two ways that turned into a silently wrong diagram:

- Inline node definitions on an edge line were mis-parsed. For
  `start([Start]) --> check_user{Is user authenticated?}` the edge parser
  failed, the node parser then matched whichever shape came first in its
  pattern list, and the edge plus the remaining node vanished.
- Non-canonical link spellings were dropped whole. gpt-4o-mini emits
  `check_user --|Yes|--> show_dashboard` (valid Mermaid is
  `check_user -->|Yes| show_dashboard`); the line went, and with it the
  `check_user{...}` decision node it referenced, so a diagram that should
  hold a DECISION node held none.

The parser now:

- resolves each edge endpoint separately, keeping inline node shapes
- normalizes the `--|label|-->` drift, plus the valid `-- label -->` and
  `-. label .->` forms
- parses chained edge lines (`a --> b --> c`)
- reports what it cannot read instead of dropping it: such lines land in
  MermaidParser.unparsed_lines and raise a MermaidParseWarning, or a
  ValueError under the new strict=True
- recognizes comments, subgraph/style/classDef directives and the legacy
  `graph` keyword, so valid Mermaid does not trip that new warning

Node shapes are now one table driving both the node and endpoint patterns.

LLMConverter routes its three parse sites through _parse_mermaid, which
refuses output holding no valid diagram instead of returning an empty one.

Claude-Session: https://claude.ai/code/session_01VipiLaG4xy7WctqY9w2475
Both real-API tests ran on mere presence of OPENAI_API_KEY, so any developer
or agent with a key exported silently made non-hermetic, network-dependent,
cost-incurring gpt-4o-mini calls -- and inherited their model drift. Require
an explicit opt-in instead.

CI behaviour is unchanged: it sets neither variable, so both tests were and
remain skipped there.

Claude-Session: https://claude.ai/code/session_01VipiLaG4xy7WctqY9w2475
- license: the [project.license] table carrying the non-SPDX value "mit"
  becomes the inline SPDX string license = "MIT". No License:: trove
  classifier is added alongside it -- PEP 639 rejects that combination.
- authors: was empty, so PyPI showed no author.
- classifiers: none were declared; the 3.10/3.12 pair mirrors the CI matrix.
- .editorconfig: added, copied from the fleet template.
- ci.yml: dropped two stale migration comments. No setup.cfg exists, and
  PYPI_PASSWORD is provisioned -- Publish has already succeeded.

Repo metadata set out of band: homepage -> the gh-pages site, plus the seven
topics mirroring the pyproject keywords.

Claude-Session: https://claude.ai/code/session_01VipiLaG4xy7WctqY9w2475
@thorwhalen
thorwhalen merged commit e968720 into master Jul 30, 2026
12 checks passed
@thorwhalen
thorwhalen deleted the wads/fix-mermaid-parser-and-metadata branch July 30, 2026 17:53
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