Repository navigation
feat(sequence): add opt-in column_fit for viewBox-relative lanes - #64
Conversation
|
Thank you for the contribution! I tested the PR merged with the current Before we merge, could you please address one blocking packaging issue and two small follow-ups? Blocking: rebuild
|
A sequence diagram always places participants with a fixed 108px gap and an 86px box, so widening meta.viewBox adds dead canvas on the right instead of column distance, and longer participant labels fail validation even when the canvas has room for them. meta.column_fit: "spread" derives the gap and the box width from the viewBox while keeping the side margin and the existing overflow check. The default "fixed" reproduces the previous constants exactly, so authored diagrams and goldens are untouched.
e60a25f to
80c9147
Compare
|
All three addressed. Rebased onto
Zip — verified the gate the way CI does: fresh build vs extracted committed archive is a clean Test — same document, same viewBox, twice: Diagnostic —
Tests: 626/626 (621 on |
tt-a1i
left a comment
There was a problem hiding this comment.
Thanks again for the focused contribution and for addressing the packaging, regression, and diagnostic feedback. The final branch also documents when agents should opt into spread. Local CJK/multi-viewport review passed, and all nine CI jobs are green.
Problem and value
A sequence diagram always places participants at a fixed 108px gap with an 86px box, so
meta.viewBoxwidth has no effect on the lanes. Two consequences:Label "…" (~116px) is wider than the 86px participant box — shorten it), so a label has to be shortened even when the canvas has plenty of room. This hits CJK labels especially hard, where a short word is wide.column_fit: "spread"lets an author trade that unused width for column distance and label room without touching lane order, IDs, or message semantics.Scope
meta.column_fit("fixed" | "spread") to the sequence schema; the sequence renderer derivesparticipantW,colGap, andleftXfrom the viewBox whenspreadis set; documented the field inschemas/README.md; addedtest/sequence-column-fit.test.mjs.fixedreproduces the previous constants exactly (participantW: 86,colGap: 108,leftX: 62), so authored diagrams, goldens, and packaged examples render byte-identically. The existing overflow check (Participants exceed viewBox width) still runs in both modes. No other renderer was touched.renderers/sequence/render-sequence.mjs,schemas/sequence.schema.json,renderers/shared/generated-validators.mjs(regenerated),schemas/README.md, and the new test.Stability impact
fixedpath.metakeepsadditionalProperties: false, so the enum is the only accepted spelling.generated-validators.mjswas regenerated withnpm run generate:validators. No template change, so no example, gallery, orarchify.zipartifact drifts.spreadstill fails closed through the existing width check when the participants cannot fit the authored viewBox. Removing the field, or reverting the commit, restores the previous behavior with no document changes.Tests run
Baseline on
upstream/mainat v2.14.0 in the same environment was 621 passing, so the five tests in this branch are the only delta.The tests assert through the public renderer seam, not private helpers:
fixedkeeps the 108px gap and 86px box for a 1320px viewBox, and the lanes stay packed on the left.spreadwidens the box past 86px, spreads the gap past 108px, keeps the 62px side margin, and keeps the last lane insideviewBox[0] - 40.column_fitrenders byte-identically to an explicit"fixed".fixedand renders unshortened inside the participant box underspread— same document, same viewBox.spreadand never quotes the fixed layout.Visual evidence
Measured from the rendered SVG for the same five-participant document at
viewBox: [1320, 620]:fixed(default, unchanged)spreadThe reserved right margin is
1320 - 40 = 1280, sospreaduses the canvas exactly to the documented limit. Local visual review passed: lanes are evenly distributed and no label is clipped. Deterministic geometry is given above rather than replaced by a screenshot.Generated artifacts
renderers/shared/generated-validators.mjsregenerated vianpm run generate:validators, andarchify.ziprebuilt viascripts/build-zip.shand committed — the schema and the renderer both ship inside the installed Skill, so the archive is part of this change. Verified locally against the same gates CI runs: a fresh build and the extracted committed archive are a cleandiff -r, andscripts/package-smoke.mjspasses on the extracted tree.assets/template.htmlis untouched, soexamples/,docs/, the Gallery, and the README proof stay fresh — confirmed bynpm test, which includes the golden and package freshness checks.Checklist
npm testinarchify/.