Repository navigation
fix: measure sublabel and tag in every renderer - #56
Conversation
`sublabel` and `tag` render as a single unwrapped <text> element with
text-anchor="middle". Only `workflow` measured its sublabel against the
node box; `architecture`, `sequence`, `dataflow` and `lifecycle` emitted
all of them at a hardcoded font size with no width check, so an over-long
value painted across its neighbours while `validate --quality showcase`
still reported 9/9 with zero errors. `workflow`'s own `tag` had the same
gap.
Lift workflow's existing two-part pattern into renderers/shared/text-fit.mjs
and apply it everywhere:
- fittedNodeFontSize shrinks the text toward a legible minimum at render
time, so ordinary overruns get smaller instead of overlapping.
- minimumNodeTextWidth reports the width still needed once the text has
shrunk as far as it may, so validation rejects what shrinking cannot
save.
The fitting geometry is shared; per-field preferred/minimum sizes stay with
each renderer, because architecture sets sublabels at 9px and the rest at
7px. Workflow's label and sublabel behaviour is unchanged.
Sequence participants are a fixed layout width with no author-facing knob,
so its message asks only for shorter text.
Every renderer's too-wide-label message advised "move detail to sublabel".
That field is measured now, so the advice would move the problem rather
than fix it; each message now asks to shorten the label or widen the box.
Adds 13 regression tests: a validation case per renderer per field, a
shrink-to-fit render case, and a contract test that no renderer redirects
an over-long label into sublabel.
The release-delivery workflow example carries a tag ('owner: on-call')
that is fractionally too wide for its node, so the new fit renders it at
6.6px instead of 7px. Only that one artifact changes.
The archive ships the renderers, so the new shared text-fit module and the four updated renderers have to be in it. Built with scripts/build-zip.sh; verified against CI's own staleness gate (build fresh, diff -r against the committed archive) and package-smoke.
|
A note on the red checks, since two of them have different causes.
$ (cd /tmp && /repo/scripts/build-zip.sh fresh.zip)
$ unzip -q /tmp/fresh.zip -d /tmp/fresh && unzip -q archify.zip -d /tmp/checked
$ diff -r /tmp/fresh/archify /tmp/checked/archify
$ echo $?
0
$ node scripts/package-smoke.mjs /tmp/pkg/archify
package smoke passed on linuxPlease treat that commit with the usual suspicion you'd give any regenerated
$ git checkout main && cd archify && npm ci && npm test
> archify@2.13.0 check:release-identity
> node ../scripts/check-release-identity.mjs
release identity: README_EN.md must remain byte-identical to README.md.
For what it's worth, with that gate stepped over, the suite on this branch shows |
|
Thank you, @pruuk, for this thoughtful and thorough contribution. The shared text-fitting implementation, actionable diagnostics, regression coverage, and detailed validation evidence make Archify more reliable across all five renderers. We've merged the PR into Thanks again for helping make Archify better! |
2 participants
fix: measure sublabel and tag in every renderer
Problem and value
sublabelandtagrender as a single unwrapped<text>element withtext-anchor="middle". Onlyworkflowmeasured itssublabelagainst the nodebox. The other four renderers emitted both fields at a hardcoded font size
with no width check, so an over-long value renders across its neighbours while
validate --quality showcasestill reports 9/9 checks, 0 errors, 0 warnings.workflowarchitecturesequencedataflowlifecycleTwo things make this worse than a cosmetic gap:
labelfield is measured; thefields next to it are not, so the receipt is green while text visibly
overlaps.
message said some form of "shorten the label, move detail to sublabel" —
advice that moved text into the one field nothing measured.
Reproduction on
main(before)The rendered SVG has that sublabel running across the neighbouring states.
Same command on this branch (after)
Scope
What changed
renderers/shared/text-fit.mjs. This isrender-workflow.mjs's existingfile-local pattern lifted into shared code, unchanged in behaviour:
fittedNodeFontSizeshrinks text toward a legible minimum at render time,so ordinary overruns get smaller instead of overlapping.
minimumNodeTextWidthreports the width still needed once the text hasshrunk as far as it may, so validation rejects what shrinking cannot save.
architecture,sequence,dataflow,lifecycle:sublabel(andtagwhere the type has one) now use
fittedNodeFontSizefor the<text>font size, and carry a
minimumNodeTextWidthcheck beside the existing labelcheck.
workflow: same treatment for itstag, which had the same gap. Itslabeland
sublabelbehaviour is byte-identical to before.They now ask to shorten the label or widen the box.
The fitting geometry is shared (
widthFactor: 0.6,horizontalPadding: 8);per-field preferred/minimum sizes stay with each renderer, because
architecture sets sublabels at 9px and the rest at 7px. The legible minimum is
6px throughout, matching workflow's existing
sublabelMinimum.sequenceparticipants are a fixedlayout.participantW(86px) with noauthor-facing width, so shrink-to-fit applies but "widen the node" does not —
its message asks only for shorter text and names the fixed width.
What deliberately did not change
preferred font sizes, so a diagram whose text already fits renders identically.
6.2/6.6/6.8per-unit constant).
No unrelated changes: confirmed.
Stability impact
sublabel or tag that cannot fit even at 6px now fails where it previously
passed. That is the point of the fix, but it is a behaviour change for
existing specs. Diagrams whose text merely overran now render shrunk and
keep passing.
throwDiagnosticProblemspath, with the same message shape as the workflowrule that has shipped for several releases (threshold + remediation verb).
text-fit.mjsis additive and has noother callers.
Tests run
Adds 13 regression tests:
workflow: node sublabel wider than its legible minimumcase.font-sizedrops below the preferred size but stays at or above 6px.sublabel.Full suite, on a LF checkout (see caveat):
The 9 baseline failures are identical on
mainand on this branch and areunrelated to this change (repository-evidence tests needing a real origin, the
live previewbrowser test,package smokeneeding a built package, and theREADME-language checks). Two further tests —
compare failure preserves an existing trusted artifactandthe shared renderer rechecks its guarded output immediately before writing— flip between runs under the concurrent runner onboth branches; both pass in isolation on this branch.
Caveat worth flagging separately: on a Windows checkout with
core.autocrlf=true, 5 extra tests fail on unmodifiedmainbecause assertionslike
skill.match(/^---\n([\s\S]*?)\n---/)intest/skill-metadata.test.mjsassume LF. A
.gitattributeswith* text=auto eol=lfwould fix that. Nottouched here.
Visual evidence
Verified against the six real diagrams that surfaced this bug (a MUD engine's
architecture/sequence/dataflow/lifecycle set), in both their pre-fix and
hand-corrected states:
The one field that is not rejected is an architecture tag needing ~230px at 7px
in a 200px box. It now renders at
font-size="6"and fits — shrink-to-fitrescuing it is the correct outcome, and is why the shipped fix uses workflow's
approach rather than a fixed-font width test.
Zero false positives on the six hand-corrected specs.
Visual review: passed (before/after SVG inspected for the lifecycle
reproduction above and for the shrunk architecture tag).
Generated artifacts
node scripts/build-gallery.mjs docs— 3 files, 6 lines total:The only artifact change in the entire gallery is the
release-deliveryworkflow example's
owner: on-calltag, which is fractionally too wide for itsnode and now renders at
6.6instead of7. Every other bundled diagram isbyte-identical, which is the clearest evidence that this does not disturb
already-fitting text.
**
archify.ziprebuilt viascripts/build-zip.shand committed. It ships therenderers, so the new
renderers/shared/text-fit.mjsand the five updatedrenderers have to be in it. 879,783 → 882,703 bytes.
Verified against this repo's own
zip-freshnessgate before pushing — build afresh archive,
diff -rthe extracted trees against the committed one, exit 0— plus
node scripts/package-smoke.mjson the extracted package.Checklist
the new blocking rule that is the subject of the PR.
npm testruns the full gate; see the caveats abovefor the pre-existing failures and the CRLF issue.
artifact changed) and
archify.ziprebuilt, both committed.Maintainer follow-up
main, including the release-identity fix from fix: restore release identity checks #62.archify.zipfrom the LF checkout so the repository's ZIP freshness gate is deterministic.Additional validation:
Fixes #61