feat(layout): nested-JSON architecture diagram layout engine (auto-routing) + MCP tool#201
Merged
Merged
Conversation
added 30 commits
June 27, 2026 11:56
- Align single leaf icons to the Y-center of sibling groups' leaves (not the group bounding box center), eliminating padding-induced offset - Align corresponding leaves across vertical groups at any depth so that horizontally connected nodes share the same Y coordinate - Reduce SNAP_THRESHOLD to 5px and remove mid-point averaging so connectors always start/end at the icon center - Make fan-out/fan-in port merging opt-in via "fan": "merge" on connections (default: separate ports per connection)
- Add optimize_order() pre-processing that sorts leaf nodes within vertical/horizontal groups by their connection source position, eliminating edge crossings automatically - Add _spread_overlapping_bends() post-processing that detects elbow connectors with bend points at nearly the same X coordinate and spaces them apart (20px step) to prevent visual overlap
- Add _segments_cross() that detects perpendicular crossings AND collinear overlap (parallel segments on the same axis with >5px shared range) - Add edge-edge crossing warning in cmd_layout output without skipping shared-node edges (shared endpoints don't prevent mid-segment crossings) - Fix KeyError when group has no label in size warnings
…arch - optimize_order now uses brute-force permutation search (≤7 leaves) with actual crossing count, falling back to heuristic for larger groups - Fix circular reference in peer position estimation: external peers now get fixed positions based on connection order, independent of the permutation being tested - Correctly minimizes edge crossings in groups like [Aurora, SQS, DynamoDB, ElastiCache] where simple position-average sorting fails
- Add _optimize_port_order() that tries all permutations of port indices (≤6 ports per side) and picks the assignment with fewest segment crossings among edges sharing that port group - Reduces edge-edge crossings from 3 to 2 in the complex feedback loop test case (remaining crossings require path detour routing) - Includes heuristic fallback for nodes with >6 ports on one side
- Replace fixed-direction shift with exhaustive candidate search: for each crossing, try both edges × ±10/15/20/30/40/50px × X/Y axis - Score candidates by (total_crossing_count, displacement) to pick the shift that minimizes crossings with least movement - Iterate up to 30 times until zero crossings remain - Resolves all edge-edge crossings in the GenAI chat app architecture (previously 3 unresolvable crossings)
…vements - Reverse-flow connections (right-to-left) now route as U-shaped polyline detours below all nodes instead of crossing through them - Reverse connections use dedicated bottom ports, excluded from normal port_counts to avoid disturbing forward-flow port positions - Polyline output for detour paths (uses existing freeform builder) - Separate close horizontal segments by adjusting bend X positions - Align same-source bends to a single X for clean tree-branch appearance - Expand bend candidate range to ±120px for larger diagrams - Add _MIN_BEND_SEPARATION constant (40px) for visual clarity
- Force all forward connections from the same source to exit from the same side (decided by the first connection's _auto_sides result) - Correct dst_side when src_side is overridden (e.g. right→top becomes right→left) to ensure arrows enter target icons from the correct side - Skip _align_same_source_bends for fan-out patterns (where start Y positions differ) to prevent horizontal segment overlap
- Fix _align_leaves_to_sibling_centers fallback: use the nearest sibling group's leaf center median for perfect alignment - Browser now sits exactly at WAF group's vertical midpoint (0px diff) - user→waf2 becomes a straight line (no L-shape from rounding error)
- _apply_bend_shift_x/y now skip points[0] and points[-1] (start/end) which are anchored to icon port positions - _apply_bend_shift_y additionally skips if target_y matches start/end Y to prevent creating diagonal segments from port-adjacent points - Fixes Lambda→Transcribe start Y being shifted from icon center and prevents non-axis-aligned (diagonal) segments in elbow paths
…e diagrams - Add constraint-based graph layout engine (skill/sdpm/layout/graph.py) - Fix reverse_set: skip when explicit side hints provided, require horizontal displacement > vertical * 0.5 to avoid treating vertical connections as reverse - Fix decided_src_side: skip consistency override when explicit sides provided - Support srcSide/dstSide hints in connection spec for _layout_route_connections - Add internal_order_constraints to optimize_order: preserve order of nodes connected within the same group (prevents SDK→NW becoming NW→SDK) - Fix KeyError when group has no 'label' in imbalance warning
- Clamp bentConnector4 adj1 to minimum 0.15 when direction is forward (prevents H-V-H connector from routing behind the source node) - Fix decided_src_side: only apply consistency override when decided side is on the same axis as the natural side (prevents forcing vertical side on a horizontal connection) - Fix reverse_set: require horizontal displacement > vertical * 0.5 to treat as reverse flow (prevents vertical connections from being routed as U-shaped detours)
When _spread_overlapping_bends shifts intermediate points, the first segment may become diagonal (neither purely vertical nor horizontal). In this case, fall back to using overall dx/dy ratio to determine whether bentConnector3 (V-H-V) or bentConnector4 (H-V-H) is appropriate.
…visual groups Containers without groupType or label (purely structural nesting) now use minimal padding/margin (5px) instead of the full group defaults (70px top padding, 20px margin). This prevents excessive spacing between blocks that don't render a visible group box.
LLM can now specify the rendering area directly in the layout JSON:
"targetArea": {"x": 960, "y": 120, "width": 880, "height": 880}
This allows placing diagrams in a specific region of the slide (e.g. right
half for text+diagram layouts). JSON values are used as defaults when CLI
args are not provided.
Fan-out sources (one node connecting to multiple targets) now always use the same exit side for all connections, even when some targets are on a perpendicular axis. This prevents arrows from exiting different sides of the source icon (e.g. one from "right" and another from "top"), which caused visual artifacts and icon overlap.
… preference Fan-out sources now pre-compute the best exit side before processing connections. The side is chosen by: prefer "right" if any target is to the right, then "left", then majority vote. This prevents the first connection from locking a suboptimal side (e.g. "top") that then gets forced onto all subsequent fan-out connections.
…pread Fan-out connections (one source → multiple targets from right side) now use a shared bend X position (45% of distance to nearest target). This creates the "trunk + horizontal branch" pattern instead of independent diagonal elbows. Fan-out edge points are saved before _spread_overlapping_bends and restored after, preventing the crossing/separation logic from breaking the shared bend alignment.
- Fan-out connections now force dst_side to the opposite of src_side (e.g. src=right → dst=left) instead of keeping the natural auto_sides result that may point to wrong edge (bottom instead of left) - Fan-out edges with 4+ points are rendered as polyline (freeform) instead of bentConnector4, avoiding adjustment calculation errors that caused arrows to miss target icons
…interiors - All nodes are now added to the obstacles list (with _node marker to exclude src/dst from their own connection's obstacle check) - _calc_bend now detects when bend position falls inside an obstacle (not just near its edge) and shifts to the nearest outside edge - Connections exclude their own src/dst nodes from obstacle avoidance to prevent self-interference
bentConnector4 adjustment calculations introduce errors that cause arrows to appear non-perpendicular to icon edges. All paths with 4+ points are now rendered as polylines (freeform shapes) which exactly follow the routing engine's computed points.
After _spread_overlapping_bends may shift bend positions into node areas, a new _fix_bends_inside_nodes pass checks intermediate segments and shifts any vertical/horizontal segment that passes through a non-src/dst node's bounding box to the nearest outside edge. Only intermediate segments are checked (start/end points are anchored to ports).
Added final pass that snaps any remaining diagonal segments to axis-aligned. After _spread_overlapping_bends and _fix_bends_inside_nodes may introduce diagonal segments by shifting intermediate points, this pass forces all segments to be either horizontal or vertical by snapping coordinates to match adjacent points.
…t icons Added final pass that ensures: - Arrows exiting from "right" side never bend left of their start point - Arrows entering from "left" side never approach from the right - Same for top/bottom variants This prevents the pattern where _spread_overlapping_bends shifts a bend to the wrong side of the source/target node, causing the arrow to pass through its own icon.
When computing fanout_bend_x, only consider targets whose left edge is to the right of the source's right edge. If all targets are to the left (e.g. Planner is below and at same X as Step Functions), use a fixed offset (src_right + 30) to ensure the bend is always forward from the source exit point.
…below When a fan-out target is mostly above/below the source (vertical distance > 2x horizontal distance), use the natural _auto_sides dst_side instead of forcing "left". This prevents the fan-out H-V-H path from crossing through the target icon when it's at the same X as the source. Also restrict fan-out shared bend path to only apply when dst_side="left" (horizontal entry), falling back to normal _elbow_path for vertical entry.
- All paths with 3+ points are now rendered as polylines for precision - Diagonal elimination pass now runs up to 5 iterations to handle cascading fixes where fixing one segment creates another diagonal - Fixed fan-out bend_x to only consider targets to the right of source
Replace the iterative snap-based diagonal elimination (which could oscillate forever) with a deterministic point-insertion approach: for any diagonal segment, insert an intermediate point that creates an L-shaped (horizontal then vertical) connection. This guarantees zero diagonal segments in a single pass without oscillation. Also improved _fix_bends_inside_nodes to maintain axis-alignment of adjacent segments when shifting points.
…below When a fan-out target is mostly above/below the source (ady > adx * 2), both src_side and dst_side now revert to natural _auto_sides values. This allows SF→Planner (directly below) to use bottom→top as a straight vertical line instead of the forced right→left fan-out pattern.
Replace fixed-threshold side detection with closest-edge calculation for backwards arrow fix. Also handle edge case where top-side port has a horizontal backwards segment.
ml_inference surfaced this: three workers each writing to both a feature store and a prediction stream put two fan-in bundles on one shared trunk, so every spoke crosses the other bundle (11 crossings). Document it as an anti-pattern with the fix — group the two targets and fan into the box, or give each target its own row.
…oint arrows Two independent improvements to the ordering/routing engine: 1. Routing speed (bbox pruning). _count_all_crossings and _count_node_pierces now reject far-apart edge/edge and edge/node pairs by bounding box before the O(segments²) segment test, and precompute each node's short id + pierce box once instead of per-pair. microservices' single full route drops from ~10.8s to ~2.1s; results are byte-identical (pure speedup). 2. Order model straightens a lone group-endpoint arrow. The crossing-order search now breaks ties by a peer-detour cost: when EXACTLY ONE direct child of a group connects outside it, seat that child at the row end facing its peer so its arrow runs straight instead of detouring around a sibling icon (the multi-region-DR "API container → Data tier" case, now a straight line). Group ids are given positions for this tie-break ONLY (via include_groups), so many-to-one edges to a sibling box are visible; the crossing count itself stays leaf-only to avoid reshuffling unrelated diagrams. Gated to the single-connected-child case so multi-way groups are left to the crossing model. All 11 demo diagrams keep identical crossings/pierces/group_pierces; only the DR diagram changes, for the better. optimize_order stays ~1ms (no per-order re-routing — an earlier full-routing-eval version was reverted as too slow).
Extract cmd_layout's inline pipeline into reusable pure functions so the
CLI, the QA harness, and (future) MCP tools all call one source of truth
instead of re-implementing scale/route/measure.
- Add sdpm/layout/render.py: build_layout() + render_architecture() ->
{elements, bbox, warnings, metrics}. Respects the root `reverse` flag.
- Add sdpm/layout/metrics.py: measure/measure_layout/score, moved from
layout_qa.py.
- Move _segments_cross into sdpm.layout (builder re-exports it).
- cmd_layout is now a thin wrapper (include_metrics=False keeps stdout
byte-identical to prior versions).
- layout_qa.py drops its duplicated _build_layout and delegates to the
canonical pipeline; still re-exports measure/score for layout_search.py.
Fixes a latent bug: layout_qa's duplicate pipeline ignored `reverse`, so
QA metrics disagreed with what the CLI actually drew for reversed diagrams.
Verified against HEAD across 7 diverse diagrams: CLI stdout byte-identical;
QA metrics identical except the one reversed diagram (wire_norm/score now
reflect true geometry). ruff clean, 188 tests pass.
Register the architecture-diagram layout engine as a first-class MCP tool
so clients can render diagrams directly instead of driving it through the
generic run_python sandbox.
- L2 (mcp-local): add tools.arch_diagram, registered in server.py,
server_with_instruction.py, server_acp.py (same 1-line pattern as grid).
- L3 (mcp-server): add @mcp.tool() arch_diagram wrapping the same
sdpm.layout.render.render_architecture pipeline.
The tool takes a logical-structure JSON string plus target-area params and
returns {elements, bbox, warnings, metrics}. Including the QA metrics
(crossings/pierces/group_pierces/overflow/score) lets the agent render,
read the numbers, and self-correct the structure in one loop — previously
this needed the layout CLI plus a separate QA script.
Verified: L2 server lists arch_diagram among 21 tools with the expected
params; render path returns metrics; bad JSON returns a clean error. ruff
clean, 188 tests pass.
The layout engine now has two equivalent front-ends (the arch_diagram MCP tool and the layout CLI). Update the guide so agents on either host know how to invoke it and read the results. - arch-layout-engine.md "How to run it": present arch_diagram (preferred — returns metrics inline) and the CLI side by side; clarify targetArea override applies to both. - "Reading the output": metrics are returned inline by arch_diagram; the CLI path still uses layout_qa.py. Document overflow/score and the render → read metrics → fix structure loop. - arch-elements.md: theme is an arg (MCP) or --theme flag (CLI). The discovery loop is closed both ways: the tool docstring points to this guide via read_guides; the guide names the tool as the preferred front-end.
…ignment-improvements
A "tile pool" is a group whose children are all anonymous, frameless, leaf-only sub-columns of one orientation — pure tiling with no semantic sub-grouping (e.g. a Services group split into two unlabeled Lambda columns). Order optimization never moves a leaf between sub-columns, so the author's column split stays frozen even when regrouping would shorten wiring and seat externally-connected leaves nearer their peers. The new _reflow_tile_pools pass (end of optimize_order) enumerates the ways to partition a pool's leaves across its columns — membership only; each candidate's intra-column order is then set by the normal optimizer — re-routes each with the real engine, and keeps the best. A candidate is adopted only if it does not worsen any hard defect (overflow/crossings/pierces/group_pierces/backwards) vs the author's arrangement; wire length is a pure tie-break, so reflow can never trade a crossing for shorter wire. build_layout gains optimize=False so the reflow evaluation re-routes candidates without recursing into optimize_order. Only groups matching the strict tile-pool shape are touched (1 of 12 test diagrams); the other 11 render byte-identical. On the e-commerce diagram the engine now auto-reaches the hand-tuned layout: Orders/Payments drop to the row facing the Queue/Payment-Provider, wire 1.108 -> 0.907, 0 crossings. Membership search keeps it fast (~380ms). 260 tests pass.
_align_group_bus bundled ALL edges sharing a group endpoint onto a single box side chosen by the free-ends' centroid. When a group radiates in several directions at once — Stream Processing → Firehose (right), → OpenSearch (above), → CloudWatch (below) — the centroid landed slightly right, so the up/down edges were dragged out the right face and detoured around the box instead of leaving straight from the top/bottom edge. Now each edge is assigned to the box side its OWN free end faces, and only sides with 2+ edges are bus-bundled; an edge alone on a side keeps its natural perpendicular port. Stream Processing's three edges now exit top / bottom / right respectively as straight runs. realtime wire 1.533 → 1.116 (OpenSearch/CloudWatch straightened), 0 crossings/pierces preserved; network unchanged on every QA metric (2px coordinate shift only); other 10 diagrams byte-identical. 260 tests pass.
The builder's "Edges A→B and C→D cross" warning used a naive segment- intersection test (_segments_cross) that flagged every fan:merge shared trunk as a crossing — directly contradicting the QA metric, which excludes those structural T-junctions. Authors chasing the warning would restructure a diagram the QA harness already called clean (0 crossings). Extract _find_crossing_pairs as the single source of truth for "which edge pairs genuinely cross" (shared per-segment skip rules: collinear-trunk overlap and fan-trunk T-junctions). The builder warning now reports those distinct pairs; the QA metric _count_all_crossings keeps its segment-pair tally (the order/reflow/bend search was tuned against that magnitude, so it must NOT become a distinct-pair count). The two now agree on *whether* a pair crosses, differing only in how the total is tallied. realtime dropped 3 phantom "cross" warnings; across 16 test diagrams the builder warning now fires iff QA crossings > 0. All 12 golden diagrams render byte-identical (layout unchanged — this only touches warning text). 260 tests pass.
- The guide said the builder's edge-crossing warning is "conservative and may flag a shared fan trunk that is not a real crossing." That is no longer true (the warning now uses the QA detector) — rewrite it: the warning fires iff the QA crossings metric is > 0, and a tall/wide-group hint is advisory (if overflow is 0 the layout fits, don't restructure). - Add a "Preview the rendered image" step to How to run it: metrics catch geometry, but only the eye catches a wrong label or awkward bend. Shows the one-slide include deck + preview command (a gap a subagent hit).
The slide flow's review step (create-new-3-review) already previews every slide as a PNG and treats it as the source of truth, so a dedicated preview instruction in this guide was redundant. Point at that step instead, and keep the one-slide-deck preview only as the out-of-flow fallback (when the arch_diagram tool is used standalone).
run_python(save=True, measure_slides=[...]) returned "preview_files": [] with no error — the core render→eyeball→fix loop had no PNGs. pdftoppm zero-pads the page number to the width of the largest index, so a 1-9 page export is "page-1.png" (no padding), but the code only probed widths 6/2/3, never 1 — so every lookup missed and the list came back empty, silently. Probe widths 1-6, and when a slug still has no PNG, set preview_error with the slugs and the filenames pdftoppm actually produced, so the gap is visible instead of an empty list. Verified: a 2-slide deck now returns both PNGs.
The builder silently skips an {"type":"include","src":...} whose file is
absent (the `if include_path.exists():` has no else), so a whole diagram
could vanish from a slide with no error or warning — forcing manual
inline-expansion to even find the cause.
Add check_includes (run in _resolve_config alongside the font-size and
overlay checks): for every include element, warn if src is empty, the file
is missing, the JSON is invalid, or it expands to 0 elements. Verified on a
deck with a missing include and an empty include — both now surface as
warnings naming the slide, element index, and resolved path.
An icon label was rendered in a textbox the width of the icon (~60px) with word_wrap off, so a caption like "Cognito" broke into "Co / gni / to" — every arch_diagram icon label came out as a vertical stack of glyphs, and the deck had to be rebuilt by hand with wide textboxes. Size the bottom label box to the longest label line (~0.85em/char + pad) with a floor of the icon width, and center it on the icon so the caption stays one line and overhangs evenly. Verified on the e-commerce diagram: "Cognito", "DynamoDB", "CloudFront", "ElastiCache" etc. dropped from 4-line glyph stacks to 1-2 readable lines. Labels longer than the icon width still overhang harmlessly (centered, engine leaves margin).
The compose step named the chart guides ("when a slide has a chart, read
guides chart-bar/…") but said nothing about architecture diagrams, so an
agent building one had no push toward guides arch-layout-engine / the
arch_diagram tool — it could hand-place icon and arrow coordinates and end
up with avoidable crossings (exactly what happened to a hand-built deck
whose viewer fan-in edges crossed, which the engine routes cleanly).
Add an architecture/system/flow-diagram clause to the same Reminder: read
arch-layout-engine and build with the layout engine (arch_diagram MCP tool
or pptx_builder.py layout), falling back to hand-placement (arch-elements)
only for fine-tuning or non-flow art.
The branch carried 7 ruff findings (F401 unused imports, F841 unused locals) accumulated across the engine work — origin/main is clean, so these would trip a lint gate on the PR. Remove them: the `score`/`copy` imports and the `v_sides`/`is_fanout_edge`/`new_h_y`/`lid`/`orig` locals were all unreferenced. Pure dead-code removal — 260 tests pass, demo diagrams route identically (0 crossings/pierces).
…urpose The guide framed fan:merge as strictly for "same purpose" edges, so an agent would leave several differently-labeled arrows sharing one hub (e.g. a relay that put→DynamoDB, publish→AppSync, invoke→Runtime; or replay/broadcast/ restore all entering one viewer) as a loose spray, even though merging them into a single trunk that splits near the far ends reads much cleaner and the per-spoke labels still render. Reframe technique 2 and the anti-pattern: merge bundles edges that SHARE A HUB node (fan-out from one source or fan-in to one target) regardless of purpose; the only thing that must not merge is edges that merely cross paths without a shared endpoint. Self-protection (roll back a trunk that would pierce an icon) is unchanged.
The bottom-label width estimate counted glyphs at a flat 0.85em, which under-sizes Japanese/CJK labels (fullwidth glyphs are square) and re-introduces the one-glyph-per-line wrapping the box is sized to prevent. Weight East Asian wide/fullwidth glyphs at 1.05em instead.
…check - render_architecture: result shape, clean-chain metrics, element types, include_metrics toggle, input immutability, targetArea override - build_layout: scale-to-fit and edge collection - metrics: crossing detection on a known X, score ordering - check_includes: all warning branches (missing src/file, invalid JSON, empty expansion, absolute path, page location)
Contributor
|
Reviewed — design follows the thin-wrapper principle (L2/L3 both delegate to
Follow-up suggestions (not blocking): |
Mechanical, behavior-preserving split of sdpm/layout/__init__.py
(4,557 lines, 130+ functions) into six modules along responsibility
boundaries:
- model.py (284) node/group lookup, port geometry, elbow paths,
box-node element generation
- geometry.py (532) segment intersection, crossing pairs, node/group
pierce detection, backwards segments, port sides
- placement.py (610) scale, translate, align and collect geometry
- ordering.py (812) crossing-minimizing child order, branch promotion,
tile-pool reflow
- routing.py (1141) orthogonal edge routing + port/group-bus/fan passes
- refine.py (1268) bend optimization, side reselection, pierce
detours, bend separation, fan-trunk rewriting
__init__.py re-exports every name, so the existing import surfaces
(pptx_builder's `from sdpm.layout import ...`, and `from . import ...`
in render/metrics/graph) are unchanged.
Import graph is a DAG: routing -> refine -> geometry -> model, with
ordering and placement independent.
Verified behavior-preserving:
- render_architecture + measure output byte-identical before/after on
3 representative specs (chain, fan-merge, nested vertical groups)
- 284 tests pass, ruff clean, all import surfaces verified
Contributor
|
Pushed 1c70242 Verified behavior-preserving: This addresses the monolith concern from my earlier comment — no follow-up issue needed for the split itself. |
Generates 12 random logical-structure trees (seeded random.Random, fully deterministic, no new test dependency) and asserts invariants that must hold for ANY input: - pipeline never crashes; elements/bbox/metrics well-formed - every leaf is placed with positive dimensions (dot-qualified ids) - every connection produces an edge with >= 2 points - measure() and render_architecture() metrics agree (entry-point drift and non-determinism guard) - score() is monotone in each defect class - _segments_cross is symmetric in segment order and endpoint order, and a segment never crosses itself (300 random segment pairs) - _count_all_crossings is invariant under edge-list order Pipeline results are memoized per seed to keep the suite fast (full suite ~6s).
Contributor
|
Pushed e6e7feb That closes out the last item from my review. LGTM from my side once CI is green. |
ShotaroKataoka
approved these changes
Jul 13, 2026
ShotaroKataoka
left a comment
ShotaroKataoka
left a comment
Contributor
There was a problem hiding this comment.
Approving. Summary of the review:
Verified locally: 292 tests pass, ruff clean, engine smoke tests OK. The L2/L3 arch_diagram wrappers correctly delegate to the engine (thin-wrapper principle). Cross-layer impact checked: L2 (all 3 stdio servers import and run arch_diagram end-to-end), L3 (pip install ./skill includes all new submodules via package auto-discovery; the new guide ships via the existing s3deploy references asset), L4 (no direct dependency).
Pushed to this branch during review (please glance over these, @naohito2000):
- 01bd119 license header for graph.py
- bb4ccf6 CJK-aware label width (fullwidth glyphs at 1.05em)
- 61c946a 24 unit tests (render/metrics/includes/label width)
- 1c70242 mechanical split of the 4.5k-line
__init__.pyinto 6 modules — verified byte-identical output on 3 representative specs - e6e7feb seeded property-based tests for the routing engine
Note for cloud deployments: the tool code and the arch-layout-engine guide ship via different paths (image vs S3 references asset) — a normal deploy.sh run updates both.
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
2 participants
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.
Summary
Adds a nested-JSON architecture-diagram layout engine to the sdpm toolkit: you describe what connects to what (groups, icons, connections) and the engine computes coordinates, clusters related icons, picks arrow ports/bends, and minimizes edge crossings and icon pierces — no hand-placed coordinates. It is exposed both as a CLI (
pptx_builder.py layout) and as a first-class MCP tool (arch_diagram) that returns the routed elements plus objective QA metrics in one call.This is a large branch because it is the full build-out of one cohesive feature; the changes concentrate in
skill/sdpm/layout/(engine) with thin CLI/MCP wrappers.What's included
Engine (
skill/sdpm/layout/)render.py(build_layout/render_architecture→{elements, bbox, warnings, metrics}) so the CLI, the QA harness, and the MCP tool all call one source of truth.metrics.py(crossings / pierces / group_pierces / overflow / lexicographic score).MCP exposure
arch_diagramtool registered on L2 (mcp-local × 3 servers) and L3 (mcp-server), returning elements + bbox + warnings + metrics.Correctness / DX fixes
check_includes: a missing/emptyincludenow warns instead of silently dropping content.preview_filespopulated for <10-page decks (pdftoppm zero-pad naming).Docs / guidance
arch-layout-engine.mdguide (schema, techniques to reach 0 crossings, MCP + CLI front-ends).Testing
make test: 260 passed, 1 skipped.make lint(ruff): clean.🤖 Generated with Claude Code