Repository navigation
CAMEL-26348: camel-diagram - a route diagram image draws the steps as cards - #28575
Conversation
The card and card-dark themes draw each step of a route diagram as a card: the icon of its kind in a circle of the
color of the kind, the kind in bold (setBody) and its detail below (groovy: body.items, from the code
setBody[groovy{body.items}]). A custom color spec draws cards with style=card. The icons are the Lucide icons of the
camel-route-diagram web component, drawn with Java2D from their path data.
The themes are available in camel cmd route-diagram, camel get history --diagram, camel get error --diagram and the
camel_render_route_diagram MCP tool. The existing themes do not change.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet-bot
left a comment
gnodet-bot
left a comment
There was a problem hiding this comment.
LGTM. The card-style rendering pipeline is coherent end-to-end: DiagramColors.parse detects style=card, the layout engine and renderer both branch on colors.isCard(), and all CLI entry points (CamelRouteDiagramAction, CamelHistoryAction, ListError, RouteDiagramTools) consistently call engine.setCardStyle(colors.isCard()) before layout. The PathParser correctly implements SVG 1.1 §F.6.5 arc endpoint-to-center conversion, handles compound numbers (1.1.9 → 1.1, .9), and the CARD_EXPRESSION greedy+DOTALL match on {...} is correct for expression languages that can contain nested braces.
One minor note for future maintainability of PathParser.number(): the method accepts e/E as an exponent marker but does not verify that at least one digit follows. If ever applied to path data containing a trailing e (e.g. "1e"), Double.parseDouble would throw. Not a concern for the current hardcoded Lucide icon data (no scientific notation), but worth a comment or guard if the parser is reused.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
…file camel cmd route-diagram --output=diagram.png without --theme saves the image in the card theme. Before, the default unicode theme saved the text diagram in diagram.txt instead. A diagram in the terminal stays text by default (an image needs a terminal with a graphics protocol). The camel_render_route_diagram MCP tool draws in the card theme when no theme is given. Before, it failed with a NullPointerException: DiagramColors.parse(null) looked null up in the immutable map of the presets. A null spec is now the dark theme. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
…ram image The dark, light and transparent themes draw the steps of a route diagram image as cards, and the old style (a box filled with the color of the kind of step, with its code in one line) is removed, with the card and card-dark themes and the style=card key. So the route-diagram developer console (format=png) and the RouteDiagramDumper draw cards too, through the Theme enum they already have. camel cmd route-diagram --output=x.png and the MCP tool draw in the light theme when no theme is given. The sample image of the route diagram page shows the cards. The upgrade guide says what changes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
davsclaus
changed the title
…st renderer route-diagram-light.png and route-diagram-dark.png came with the route-diagram command (CAMEL-21975) and no page has ever shown them. They show the style of the first renderer. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
✅ Generated files are up to dateAn earlier CI run reported uncommitted generated changes; the latest run no longer does. |
|
🧪 CI tested the following changed modules:
🔁 1 test passed only after a retry on JDK 21 (1 retried attempt) Recovered flaky tests on JDK 21 (1)
🔬 Scalpel shadow comparison — Scalpel: 8 of 708 tested, 24 compile-only — current: 8 all testedMaveniverse Scalpel detected 8 affected modules (current approach: 8). Skip-tests mode would test 8 modules (5 direct + 6 downstream), skip tests for 24 (generated code, meta-modules) Modules Scalpel would test (8)
Modules with tests skipped (24)
💡 Manual integration tests recommended:
All tested modules (35 modules, 5m 25s total)Total reactor time: 5m 25s
Top 20 slowest modules:
|
… component, and read an exponent only with its digits The docs images of a component are links to its src/main/docs, so the regeneration removed the card image that was only under docs. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
|
Thanks. The same commit fixes what the Regen commit showed: the docs images of a component are symlinks into its Claude Code on behalf of davsclaus |
…n how the fonts of the platform wrap a plain box Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
gnodet-bot
left a comment
gnodet-bot
left a comment
There was a problem hiding this comment.
Following up on my earlier review — the PathParser.number() exponent guard has been addressed.
The updated number() method now uses exponentDigits(pos + 1) to verify that at least one digit follows the e/E marker before consuming the exponent. This correctly handles "1e" (the e is not consumed, the number parses as 1) and the test explicitly validates this edge case: assertThatThrownBy(() -> RouteDiagramIcons.path("M1e 2")) confirms the parser throws when e without digits reaches a subsequent command. ✅
The test improvement in this commit (String.join("", ...) instead of containsExactly) also removes a platform-dependent font-wrapping assumption, making the suite portable. ✅
LGTM.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
oscerd
left a comment
oscerd
left a comment
There was a problem hiding this comment.
Read through the layout/renderer core, the new icons and the test. This is clean, well-factored work and the card output is a nice upgrade. A few things I checked and liked, plus a couple of small notes.
Verified:
- Card-style wiring is consistent end to end.
drawNodenow always paints a card and sizes againstcomputeCardHeight, so the layout engine must agree — and it does:RouteDiagramRenderercallsengine.setCardStyle(true), while the jbang action and the MCP tool usesetCardStyle(!isTextTheme())so the ASCII/Unicode themes keep the plain node layout. No height-vs-paint mismatch. - Licensing is handled. The Lucide paths are now also inlined in
RouteDiagramIcons.java, andTHIRD-PARTY-NOTICES.txtis updated to say exactly that (ISC, Apache-compatible). Good to see the attribution kept in step with the new location. DiagramColors.parse(null)guard is a real fix, not just defensiveness:COLOR_PRESETSis aMap.of(...), sogetOrDefault(null, …)would have thrown NPE — now a null spec resolves to the dark theme, andnoSpecIsTheDarkThemepins it.- Safe-tooling posture intact. Rendering stays in-memory (
BufferedImage), the icons are inlined SVG paths with no external fetch, and nothing here reaches the network or runs route code — consistent with camel-diagram's "reads/renders without executing" contract.
Refactors read well: the fit(...) ellipsis helper removes the duplicated truncate loop, cardTitleFont centralises the bold title font, and cardLabel splitting setBody[groovy{body.items}] into setBody + groovy: body.items via CARD_EXPRESSION is tidy (and the kind.isEmpty() → info.type fallback covers the odd code). Tests are thorough and at the right level — label parsing, per-mode labels, the null-spec theme, and card layout/height — in AssertJ, which matches camel-diagram's existing convention. Docs are complete: diagram.adoc plus its catalog mirror, the jbang command page and metadata, the upgrade-guide note, and the new sample images. CI is green.
Two small, non-blocking notes:
- The 4-arg
RouteDiagramRenderer(nodeWidth, fontSizeScaled, nodeTextPadding, metrics)now ignoresnodeTextPadding(card text starts after the icon), which the javadoc says. Keeping it preserves the constructor signature, but if nothing external calls it,@Deprecatedor dropping it would avoid a dead parameter that reads as if it still does something. - This is the base of a three-PR stack (#28585 body types on arrows, #28586 marking problem steps — the latter is currently CONFLICTING). I'll look at those next; nothing here blocks them.
No blockers.
Claude Code on behalf of oscerd
|
Thanks for the review, @oscerd.
Claude Code on behalf of davsclaus |
…ructor with the unused text padding Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
oscerd
left a comment
oscerd
left a comment
There was a problem hiding this comment.
TL;DR
The deprecation is done properly and every caller in the repo now uses the 3-arg constructor; one doc gap remains for the programmatic callers the deprecation is there to protect.
Blockers: None.
Other findings: 2 (non-blocking).
Since my last review (7fb30ac → 35eef58)
- The 4-arg constructor (my note): addressed. It has
@Deprecated(since = "4.24.0")and@deprecated use {@link #RouteDiagramRenderer(int, int, boolean)}(RouteDiagramRenderer.java:111-119), in the X.Y.Z form most of the codebase uses. The three jbang callers moved (CamelHistoryAction.java:469,CamelRouteDiagramAction.java:316,ListError.java:408). No caller of the 4-arg form is left: the other callers onmainuse the no-arg and 3-arg forms, and #28585 and #28586 add none. The upgrade-guide note (camel-4x-upgrade-guide-4_24.adoc:2533-2535) is in the card-style section, and it is needed: the constructor is public in every camel-diagram release (RouteDiagramRenderer.java:100atcamel-4.21.0,camel-4.22.0andcamel-4.23.0). That is also why deprecating it, rather than removing it, is the right call. - No test calls the deprecated constructor now. Its body is a one-line delegation (
:118), so I would not ask for one. - Thanks for confirming the merge order of the stack.
- CI on 35eef58: Build and test (JDK 21), PR doc validation and PR license check are green.
Findings
1. [Important] The standalone Java API example lays out boxes, and the renderer paints cards over them (diagram.adoc:73-78, and its catalog copy)
First, a correction to my earlier note: engine.setCardStyle(true) is called by DefaultRouteDiagramDumper.java:207, not by the renderer. cardStyle is false by default (RouteDiagramLayoutEngine.java:70, javadoc at :153-157). drawNode always paints a card, at the height the layout gives it (RouteDiagramRenderer.java:543-598).
The documented example creates new RouteDiagramLayoutEngine() and new RouteDiagramRenderer() and never calls setCardStyle. So it gets the lines and heights of the old boxes (RouteDiagramLayoutEngine.java:580-583 instead of :577-579):
- the whole code, such as
setBody[groovy{body.items}], becomes the bold title, cut with an ellipsis at the card's narrower text width (RouteDiagramRenderer.java:581-582); - the kind and the detail are not split;
- at the default size, a one-line step gets a card 64 px tall instead of 84.
The callers outside Camel that the deprecation keeps compiling get the same image after upgrading, if their code is written like this example against 4.23.0.
Adding the call to the example, and one sentence to the upgrade-guide section, would cover both cases:
RouteDiagramLayoutEngine engine = new RouteDiagramLayoutEngine();
// the renderer draws the steps as cards, so lay them out as cards
engine.setCardStyle(true);
RouteDiagramRenderer renderer = new RouteDiagramRenderer();A program that lays out routes with
RouteDiagramLayoutEngineand draws them withRouteDiagramRenderermust callengine.setCardStyle(true)beforelayoutRoute. Without it, the cards get the height and text of the old boxes.
2. [Suggestion, pre-existing] The path-highlighting Java snippet calls a constructor that does not exist (diagram.adoc:442)
new RouteDiagramRenderer(nodeWidth, fontSize) matches none of the constructors (RouteDiagramRenderer.java:99, :104, :117). The same line is in the 4.23.0 docs. Since we are tidying the constructors anyway, the 3-arg form would make the snippet compile:
RouteDiagramRenderer pngRenderer = new RouteDiagramRenderer(
engine.getNodeWidth(), fontSize * RouteDiagramLayoutEngine.SCALE, false);This can go in this PR or in a follow-up. Either way, the catalog copy of diagram.adoc has to be regenerated with it.
Reviewed with Claude Code on behalf of oscerd. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
…allers Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
|
Thanks for the review, @oscerd.
Claude Code on behalf of davsclaus |
gnodet-bot
left a comment
gnodet-bot
left a comment
There was a problem hiding this comment.
Previous findings are all addressed. LGTM.
- oscerd's concern about the missing
engine.setCardStyle(true)call in the standalone Java API example (diagram.adoc): ✅ addressed — added at line 75 with a clarifying comment, and documented in the 4.24 upgrade guide. - oscerd's concern about the non-existent
RouteDiagramRenderer(nodeWidth, fontSize)constructor in the path-highlighting snippet: ✅ fixed toRouteDiagramRenderer(engine.getNodeWidth(), fontSize * RouteDiagramLayoutEngine.SCALE, false). - Both the component source (
components/camel-diagram/src/main/docs/diagram.adoc) and its generated catalog copy are updated identically.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
CAMEL-26348
The PNG image of a route diagram now draws each step as a card:
setBody);groovy: body.items, from the codesetBody[groovy{body.items}]).This replaces the old style, where each step was a filled box with one line of code.
What draws cards
The
dark,lightandtransparentthemes, wherever a route diagram is a PNG:camel cmd route-diagram,camel get history --diagramandcamel get error --diagram;camel_render_route_diagramMCP tool;route-diagramdeveloper console withformat=png;RouteDiagramDumper.The developer console and the dumper get cards through the
Themeenum they already use, so camel-api doesn't change.These don't change: the ASCII/Unicode diagrams, the topology diagram, and the web component.
Defaults
camel cmd route-diagramin the terminalunicodeunicode(unchanged: an image needs a terminal with a graphics protocol)camel cmd route-diagram --output=x.png, no--themeunicode, so it saved x.txtlight, so it saves the PNGcamel_render_route_diagram, no themelightThe MCP failure came from
DiagramColors.parse(null), which lookednullup in the immutable map of the presets. A null spec is now the dark theme.Changes
RouteDiagramIcons(new): the Lucide icons of thecamel-route-diagramweb component, drawn with Java2D from their SVG path data (a small parser for M/L/H/V/C/S/A/Z). No new dependency.RouteDiagramLayoutEngine:setCardStyle(true)lays a node out as a card: a title line, then the detail wrapped beside the icon (at most 3 lines). The label modes still apply:descriptionandbothshow the description below the kind. The text renderers keep it off.RouteDiagramRenderer: draws cards only. The old box drawing is removed. The presets gaincard,cardBorder,cardTextandcardDetailcolors, and a custom spec can set them. The constructor withnodeTextPaddingis deprecated: the text of a card starts after its icon, so the padding is not used. The camel-jbang commands use the constructor without it.diagram.adocgets a "Steps as Cards" section.components/camel-diagram/src/main/docs, with a symlink underdocs/components/modules/ROOT/images, like the other component images. (The regeneration had removed a copy that lived only underdocs.)textcolor no longer applies to route steps, the.pngand MCP defaults changed, and the renderer constructor withnodeTextPaddingis deprecated.images/jbang/route-diagram-light.pngandroute-diagram-dark.png. They came with the first renderer (CAMEL-21975) and no page ever showed them.A zoom level like the one in the TUI diagram can come later, for big routes.
Tests
RouteDiagramCardStyleTest(camel-diagram) covers:parse(null)gives the dark theme;CamelRouteDiagramActionRunningTest(camel-jbang-core) covers:.pngfile gets thelightimage and any other file gets the text diagram.RouteDiagramToolsTest.imageWithoutATheme(camel-jbang-mcp): the MCP tool renders a YAML route to a PNG without a theme. It fails on main.RouteDiagramToolsTestpasses (6).Claude Code on behalf of davsclaus
🤖 Generated with Claude Code
https://claude.ai/code/session_012nJsFsjqqFiwroeHRWaKbS