Skip to content

feat(sequence): add opt-in column_fit for viewBox-relative lanes - #64

Merged
tt-a1i merged 2 commits into
tt-a1i:mainfrom
pax-d:feat/sequence-column-fit
Aug 13, 2026
Merged

tt-a1i merged 2 commits into
tt-a1i:mainfrom
pax-d:feat/sequence-column-fit

Conversation

@pax-d

@pax-d pax-d commented Aug 11, 2026 •

Copy link
Copy Markdown
Contributor

Problem and value

A sequence diagram always places participants at a fixed 108px gap with an 86px box, so meta.viewBox width has no effect on the lanes. Two consequences:

  • Widening the viewBox adds dead canvas on the right instead of column distance. With five participants and a 1320px viewBox the last lane ends at x=537, leaving 743px unused.
  • Participant labels are validated against the 86px box (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

  • What changed: added meta.column_fit ("fixed" | "spread") to the sequence schema; the sequence renderer derives participantW, colGap, and leftX from the viewBox when spread is set; documented the field in schemas/README.md; added test/sequence-column-fit.test.mjs.
  • What deliberately did not change: the default. fixed reproduces 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.
  • No unrelated changes: confirmed. Only renderers/sequence/render-sequence.mjs, schemas/sequence.schema.json, renderers/shared/generated-validators.mjs (regenerated), schemas/README.md, and the new test.

Stability impact

  • Compatibility and migration risk: none for existing documents. The field is optional and additive, and an unset value takes the fixed path. meta keeps additionalProperties: false, so the enum is the only accepted spelling.
  • Renderer, validator, package, or generated-artifact risk: generated-validators.mjs was regenerated with npm run generate:validators. No template change, so no example, gallery, or archify.zip artifact drifts.
  • Failure behavior and rollback path: spread still 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

$ node --test test/sequence-column-fit.test.mjs
# tests 5
# pass 5
# fail 0

$ npm test
# tests 626
# pass 626
# fail 0

Baseline on upstream/main at 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:

  1. fixed keeps the 108px gap and 86px box for a 1320px viewBox, and the lanes stay packed on the left.
  2. spread widens the box past 86px, spreads the gap past 108px, keeps the 62px side margin, and keeps the last lane inside viewBox[0] - 40.
  3. An unset column_fit renders byte-identically to an explicit "fixed".
  4. A participant label too wide for the 86px box fails under fixed and renders unshortened inside the participant box under spread — same document, same viewBox.
  5. The sublabel width diagnostic reports the width in force under spread and never quotes the fixed layout.

Visual evidence

Measured from the rendered SVG for the same five-participant document at viewBox: [1320, 620]:

box width column gap first lane x last lane right edge
fixed (default, unchanged) 86 108 19 537
spread 190 257 62 1280

The reserved right margin is 1320 - 40 = 1280, so spread uses 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.mjs regenerated via npm run generate:validators, and archify.zip rebuilt via scripts/build-zip.sh and 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 clean diff -r, and scripts/package-smoke.mjs passes on the extracted tree. assets/template.html is untouched, so examples/, docs/, the Gallery, and the README proof stay fresh — confirmed by npm test, which includes the golden and package freshness checks.

Checklist

  • I used a minimal focused change and preserved existing typed JSON behavior unless the issue requires a contract change.
  • I ran the relevant targeted tests and npm test in archify/.
  • I added or updated a regression test for behavioral changes.
  • I checked generated artifacts and package freshness when their sources changed.
  • I removed secrets, private repository content, and customer data from fixtures and screenshots.

@tt-a1i

tt-a1i commented Aug 11, 2026

Copy link
Copy Markdown
Owner

Thank you for the contribution! column_fit: "spread" addresses a real Sequence layout limitation, and I especially like that it is opt-in while the default remains byte-identical to fixed.

I tested the PR merged with the current main: it merges cleanly, the focused 3 tests pass, the full suite passes 624/624, and a real CJK label that is too wide for the fixed box validates successfully with spread.

Before we merge, could you please address one blocking packaging issue and two small follow-ups?

Blocking: rebuild archify.zip

This PR changes files shipped in the installed Skill (render-sequence.mjs, sequence.schema.json, generated-validators.mjs, and schemas/README.md), but the committed archive was not rebuilt. I compared a fresh archive with the PR archive and those four files differ. More importantly, the source checkout accepts meta.column_fit: "spread", while the CLI extracted from the committed archify.zip rejects it with:

/meta must NOT have additional properties {"additionalProperty":"column_fit"}

Please run scripts/build-zip.sh, commit the refreshed archify.zip, and confirm the ZIP freshness/package-smoke checks. npm test does not itself prove committed ZIP freshness; that is a separate CI gate.

Test the user-facing value directly

The current tests verify geometry using short English labels. Please add one minimal regression showing that the same long/CJK participant label fails with fixed because the 86px box is too narrow, but succeeds with spread and remains inside the participant box. That locks in the main problem described by this PR, not only the coordinate math.

Clarify the sublabel diagnostic

In spread mode, the existing error still says participant boxes are a fixed ${layout.participantW}px. Please reword it to describe the width available for the current viewBox, so the repair guidance does not imply the historical fixed layout is still active.

Once these are updated and the fork CI is green, this should be in good shape for another pass. Thanks again for keeping the change focused and backward-compatible.

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.
@pax-d
pax-d force-pushed the feat/sequence-column-fit branch from e60a25f to 80c9147 Compare August 13, 2026 05:38
@pax-d

pax-d commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

All three addressed. Rebased onto main @ v2.14.0 (a3bf80c).

Request Done
Rebuild archify.zip scripts/build-zip.sh re-run, archive committed
Test the user-facing value new test: a wide label fails fixed, fits spread
Clarify the sublabel diagnostic message now reports the width in force

Zip — verified the gate the way CI does: fresh build vs extracted committed archive is a clean diff -r, scripts/package-smoke.mjs passes, and the CLI extracted from the committed zip now renders column_fit: "spread" (participant box width="190") instead of rejecting /meta must NOT have additional properties.

Test — same document, same viewBox, twice: fixed exits non-zero with 86px participant box; spread renders, the label stays inside the measured box, and the text is emitted unshortened. The fixture uses a long ASCII label so it carries no encoding assumptions; happy to switch to a CJK one if you prefer it reads closer to the reported case.

Diagnostic —

  • fixed: participant boxes are a fixed 86px unless meta.column_fit is "spread"
  • spread: participant boxes are 190px for this viewBox width and 5 participants

Tests: 626/626 (621 on main in the same environment, plus 5 here).

@tt-a1i tt-a1i left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@tt-a1i
tt-a1i merged commit 45f0611 into tt-a1i:main Aug 13, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants