fix(lineage): surface the ambiguity warning on every diagram surface (#584) - #588
Merged
Merged
Conversation
…oo (#584) #579 gave `lineage show` and the JSON routes an ambiguity warning, but the mermaid renderer had no way to carry one: a diagram has no metadata channel, so all three callers dropped it. A Web UI or --format mermaid user therefore still got one project's answer looking like the whole picture -- the exact shape of #568, just on a different surface. render_mermaid now takes an optional `warnings` list and emits each as a standalone, deliberately unconnected note node, styled as a warning so it does not read as part of the dependency graph. All three callers pass what the query result carries: the CLI's --format mermaid/html, the `lineage server` browser, and GET /lineage/mermaid. Warnings go through the same label escaping as every other API-derived string (issue #269 sec-05), covered by a test. Also removes _find_node, which #579 left behind with no callers. Leaving it invites a future caller to bypass candidate resolution entirely, which is the bug #568 reported.
… from it Two follow-ups from review of the flowchart fix. render_er_diagram had no warnings channel either, so the ER view dropped the note on both HTTP surfaces (the CLI's --format er was covered only because the warning goes to stderr before rendering). erDiagram has no free-standing annotation, so the notes ride as one relationship-less "note" entity; existing html.escape(quote=True) neutralises the quotes that would otherwise terminate the attribute comment. Rendering the diagrams in a real browser then showed the remedy losing its most important part: '--upstream/--downstream <project>:id' came out as '--upstream/--downstream :id'. Escaping is correct and holds -- mermaid renders the escaped entity back into SVG text as a literal <project>, which the browser drops as an unknown tag. It looks right in a terminal and passes every string assertion, and is wrong only where a user reads it. The placeholder is now PROJECT:, with a test asserting warning text stays free of angle brackets.
padak
force-pushed
the
claude/issue-584-lineage-warning-surfaces
branch
from
August 14, 2026 09:41
a8ae482 to
5376ef0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #584.
What was wrong
#579 added the ambiguity warning for an unqualified table id that resolves to several projects, but only the text surfaces got it. Neither mermaid renderer had a channel to carry a warning, so every diagram caller silently dropped it — a Web UI or
--format mermaid/eruser still saw one project's answer presented as the whole picture. That is #568 again, on other surfaces.What changed
Flowchart (
render_mermaid) takes an optionalwarningslist and renders each as a standalone note node, styled as a warning and deliberately left unconnected so it reads as a note rather than a graph participant.ER (
render_er_diagram) got the same channel.erDiagramhas no free-standing annotation, so the notes ride as one relationship-less"⚠ note"entity. The existinghtml.escape(quote=True)neutralises the double quotes that would otherwise terminate the attribute comment.All call sites now pass
result.get("warnings"): CLI--format mermaid/html/er, thelineage serverbrowser, andGET /lineage/mermaidin both views._find_noderemoved — #579 replaced it with_find_node_candidatesand left it with no callers. Keeping it invites a future caller to bypass candidate resolution, which is the original bug.The bug only a real browser could show
Rendering both diagrams through mermaid revealed the remedy losing its most important part:
<project>was gone. Escaping is correct and holds — mermaid renders the escaped entity back into SVG text as a literal<project>, and the browser then drops it as an unknown tag. It looks right in a terminal, passes every string assertion, and is wrong only where a user actually reads it.The placeholder is now
PROJECT:, with a test asserting warning text stays free of angle brackets. Verified by re-rendering: both views now showPROJECT:in.c-shared.ordersin full.Tests
8 new tests: warning reaches both renderers, no note when there is nothing to warn about, escaping holds in both (
<script>cannot survive, quotes cannot break out of the ER attribute comment), multiple warnings are numbered, and the angle-bracket regression guard.ruff,ty, and the lineage + serve suites (372 passed) are clean.