Skip to content

generate_sequence_diagram silently returns LLM output whenever context is non-empty, despite being labelled a Tier 2 AST tool #312

Closed

Description

@piyushmakhija28

Defect

UMLDiagramGenerator.generate_sequence_diagram(call_chains=None, context="") in langgraph_engine/diagrams/legacy_generator.py: when context is non-empty, the AST-built Mermaid sequence diagram is handed to self._llm_enrich(...) and the LLM's output is returned instead of the deterministic AST rendering.

mcp-uml-diagram/server.py's generate_sequence_diagram tool passes context=entry_function — i.e. its entry_function parameter ("Optional function name to trace the call chain from") is silently repurposed as LLM-enrichment context. There is no actual entry-function tracing/filtering implemented anywhere in the method; passing a non-empty entry_function does nothing but trigger LLM enrichment.

Net effect: any caller who supplies entry_function to what is documented and labelled a "Tier 2 AST" tool gets an LLM-authored diagram instead, with no indication in the response that this happened.

Why this wasn't fixed alongside #311

Found while implementing the UDM caller-supplied-data feature (#311) and its ADR — flagged there as a fourth rule-45-style violation but explicitly out of that PR's scope, since #311 is about the 8 LLM-only diagram types and this is a fifth, previously-believed-deterministic one with a different root cause (a tool-layer parameter mix-up, not a missing deterministic renderer).

Suggested fix

Either:

  • give entry_function real meaning (filter/trace call_chains to those reachable from the named function) and stop passing it as context=, or
  • if LLM enrichment via a context hint is still wanted, expose it as its own explicitly-named parameter (e.g. enrichment_hint) so a caller can tell, from the tool's own signature, that a non-empty value changes the diagram from deterministic to LLM-authored.

Either way, the response dict should indicate which path was taken (see the "source" key added to every tool in #311's implementation, for the same reason).

Evidence

Read directly from legacy_generator.py's generate_sequence_diagram and mcp-uml-diagram/server.py's generate_sequence_diagram tool this session.

Activity

  1. piyushmakhija28 commented on Sep 7, 2026

    @piyushmakhija28
    CollaboratorAuthor

    Fixed in b7c40f6 (this repo, PR #317) and 79d703e (mcp-uml-diagram, PR #5).

    entry_function is now a real BFS filter on caller→callee reachability (_filter_chains_from_entry), and the context-triggered _llm_enrich() path is removed entirely from both generate_sequence_diagram and _sequence_from_fqn_chains. The mcp-uml-diagram tool call site now passes entry_function=entry_function instead of context=entry_function.

    4 new regression tests added (TestSequenceDiagramEntryFunctionFilter), including an explicit "must never reach an LLM call" guard. Full relevant suite: 106/106 pass.

    Went with Option A from the issue body (give entry_function real meaning) rather than Option B (a separate enrichment_hint param) — confirmed via grep that nothing else in the codebase used the LLM-enrichment path intentionally, so there was no real feature to preserve behind a new parameter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingnot-implementedA needed capability does not exist yet

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions