feat(erd): add entity-relationship diagram type (#115)

[akyourowngames]

Summary

Implements #115: a sixth typed diagram erd for entity-relationship maps (entities, attributes, primary/foreign keys, relationship cardinality) as self-contained HTML with deterministic SVG, following the existing renderer pattern.

Rebased onto current main (was CONFLICTING, now MERGEABLE). All findings from both owner reviews addressed (details in reply below).

User problem /-linked issue

Closes #115. The five existing types cannot state data shape (cardinality, keys, constraints).

What changed

• Schema archify/schemas/erd.schema.json: entity kinds, attribute roles, FK references, both-end cardinality, repository + sources[] evidence shape (1–3 items, same bound as architecture).
• Renderer archify/renderers/erd/render-erd.mjs: fail-closed validation (cardinality both ends, FK must reference real entity.attribute, at most one PK, no duplicate attribute names, orphan incl. zero-relationship case, enforced_by resolves), cx/cy centres always in sync, per-entity authored-pos preservation, crow's-foot markers.
• guide routing: new data-model recipe (archify/recipes/scenarios.mjs, en+zh, proof billing).
• --repo-root evidence gates: repository-evidence.mjs generalized to per-type surfaces (components/entities); CLI gate extended (inline list so doctor still runs from incomplete installs); help text updated.
• SKILL.md type router + authoring invariant; bin/archify.mjs, bin/preview.mjs, shared/cli.mjs, shared/i18n.mjs list erd.
• Examples: archify/examples/billing.erd.json (+HTML in both archify/examples/ and examples/).
• Canonical archify.zip rebuilt with Node 22 (includes ERD surface; package smoke-tested outside repo).

Deliberately not changed

• No shared-geometry change: an earlier WIP touched labelPoint for all types and regressed the architecture web-app golden, so it was reverted. The ERD billing fixture instead carries explicit labelDx/labelDy on the via relationship (ERD-scoped fix).
• No auto-layout, no schema introspection, no Delta for ERD, no version/tag changes.

Compatibility / migration

Purely additive: new type keyword, schema, renderer, recipe, examples. Existing five schemas, validators, artifacts, and viewer contract untouched (architecture golden passes unchanged).

Failure behavior / rollback

Invalid ERD fails non-zero with machine-readable diagnostics; deliver preserves previous output on failure. Rollback: revert the 5 commits.

Tests and final-artifact evidence

• node --test test/erd.test.mjs 16/16 (new: FK-without-references fails closed, orphan-with-relationships:[] fails closed, finite-coordinate render check, authored-pos preservation).
• test/guide.test.mjs (12 recipes incl. data-model, en+zh routing), generated-artifact-xml 3/3, test/golden.mjs all checks passed, check:validators / check:brand-marks / check:release-identity exit 0.
• test/cli.test.mjs 40/41, test/repository-evidence.test.mjs 8/9 in parallel run; the 2 remaining failures are Windows-only preview-server lifecycle issues (SIGTERM kill semantics, 20s port contention) that pass in isolation and also fail on unmodified upstream/main code paths untouched by this PR.
• deliver erd billing --quality showcase: 9/9 checks, 0 errors, 0 warnings (spec sha256 9601b04b…, artifact 55b9da75…, 705101 bytes).
• visual-check: pass with real Chrome, 0 diagnostics (perceptual review still pending a human).
• Package smoke: extracted archify.zip outside repo renders + validates billing and routes guide→data-model (high confidence).

Regenerated files

archify/examples/billing.erd.html, examples/billing.erd.html, archify/renderers/shared/generated-validators.mjs, archify.zip.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[prashantpiyush1111]

I reviewed the current PR diff and changed files. The ERD implementation itself is substantial and the PR is marked mergeable, but I found one blocking scope/repository-hygiene issue: New Text Document.txt is an unrelated file containing a GitHub/fork instruction. It is not part of the ERD feature and should not be committed to the repository. Please remove it and re-run the relevant checks before merge.

[tt-a1i]

Code Review Summary

Verdict: Changes Requested

Thanks for taking on a substantial and valuable addition. A typed ERD fits the project direction, but the current head has correctness and distribution blockers that need to be resolved before merge.

Blocking findings

1. The generated ERD SVG is not well-formed XML. In archify/renderers/erd/render-erd.mjs:607, data-node-kind and focusNodeAttrs(...) are concatenated without whitespace. Running node --test test/generated-artifact-xml.test.mjs fails on archify/examples/billing.erd.html with 39:86: no whitespace between attributes.

2. Foreign-key validation fails open. At lines 447-455, an attribute with role: "foreign" and no references value is accepted because validation only runs when references already exists. A direct render probe exits 0 and writes an artifact. Please require a valid reference whenever the role is foreign, and add a regression test for the missing-reference case.

3. Orphan validation fails open when there are no relationships. Lines 474-485 wrap the entire orphan check in if (connections.length > 0). Setting relationships: [] therefore accepts every non-standalone entity and renders successfully. The orphan rule should also apply to the zero-relationship case, with a committed regression test.

4. The installed package and the Issue #115 acceptance surface are incomplete. archify.zip contains no ERD schema, renderer, or example, and its CLI still lists only the original five types. Issue #115 also states that guide routing should learn ERD and repository evidence should reuse the existing --repo-root gates; the PR explicitly defers both. Please complete these surfaces in this PR, rebuild the canonical ZIP, and run package smoke tests.

5. Remove the unrelated New Text Document.txt. It contains only a fork instruction and is not part of the ERD feature.

After these fixes, please run the full required suite and attach the current CI/package evidence. The feature itself is worth continuing; this review is about making the typed, fail-closed, distributed contract match what the PR and issue promise.

[akyourowngames]

@tt-a1i on it!!

[tt-a1i]

Additional exact-head review at b32c99a. The previous XML/FK/orphan/package/scope findings were rerun and still apply; this adds newly isolated failures rather than repeating those requests.

[P1] Populate the centers required by the shared router (archify/renderers/erd/render-erd.mjs:86). Entity rectangles contain x/y/width/height but never compute cx/cy after positioning. Shared anchor/default-side helpers require those fields. All six relationships in the official billing fixture emit undefined/NaN coordinates. Reproduced with node archify/bin/archify.mjs validate erd archify/examples/billing.erd.json --json: exit1, artifact/finite-svg. A two-entity probe likewise renders an unusable M undefined 152 L undefined 152. Add finite-coordinate and public validate/deliver regressions, not only marker/golden string checks.

[P2] Do not discard explicit positions when another entity omits its position (:90 onward). With standalone A at[400,300] and B lacking pos, entities.every(pos) chooses auto-grid for all entities and moves A to[40,40]. This silently overrides authored geometry and contradicts the typed/manual-layout contract. Preserve authored positions or reject missing required geometry; do not rewrite all nodes.

ERD tests12/12 and golden checks pass while reproducing broken geometry; the separate XML test still fails. Source-generated validators pass. These passes are explicitly not a correct final-artifact acceptance. No edits or merge.

[akyourowngames]

@tt-a1i all findings from both reviews are addressed in the rebased head (987667c, mergeable). Point by point:

Review 1 (Aug 29)

1. XML well-formed — fixed: renderEntity no longer duplicates data-node-id/data-node-kind alongside focusNodeAttrs(). generated-artifact-xml 3/3 green; both golden HTMLs regenerated.
2. FK fail-open — fixed: role: "foreign" without references is now an error, with a dedicated regression test (a foreign role without references fails closed).
3. Orphan fail-open at zero relationships — fixed: the connections.length > 0 guard is gone; relationships: [] fails with orphan errors unless standalone: true. Regression test added.
4. Package + Proposal: erd — a sixth typed diagram for entity-relationship / data models #115 surface — completed in-PR: CLI/preview/i18n list erd; guide routes data-model questions to erd via a new data-model recipe (en+zh, proof billing); --repo-root evidence reuses the existing gates generalized to per-type surfaces; canonical archify.zip rebuilt with Node 22 and smoke-tested outside the repo (render + validate + guide all pass from the extracted package).
5. New Text Document.txt — removed (commit dropped during rebase; file deleted from git and disk).

Review 2 (Aug 31)

• [P1] centres — placeEntity now always sets cx/cy in sync for both positioned and gridded entities. Added a finite-coordinate render regression plus public validate/deliver evidence (no M undefined, no NaN composition points; showcase deliver 9/9). Remaining undefined strings in output are viewer-runtime JS (typeof X === 'undefined'), not coordinates.
• [P2] position clobber — fixed: authored pos is preserved per entity; only entities omitting pos take grid slots (collision-aware). Regression test with A at [400,300] + B without pos.

Two judgment calls to flag

• A WIP labelPoint tweak in shared geometry.mjs regressed the architecture web-app golden, so I reverted it (no cross-type change). The ERD billing fixture instead carries explicit labelDx/labelDy on via — ERD-scoped.
• The CLI --repo-root gate inlines ['architecture', 'erd'] instead of importing repository-evidence.mjs: a static import made bin/archify.mjs unimportable from incomplete installs and broke doctor (caught by cli: doctor identifies an incomplete installation, now green). Sync is guarded by the existing evidence-shape test asserting the same message.

Residual notes (not blockers from my side)

• test/cli.test.mjs preview-server test and one repository-evidence preview test fail only under Windows parallel-load (SIGTERM kill semantics / port contention); both pass in isolation and the preview paths are untouched by this diff. Full npm test was still running locally when I pushed; CI matrix will confirm.
• visual-check passes with real Chrome; perceptual review needs a human.

Requesting re-review. Happy to split anything you would rather see separated.

[coderabbitai]

Warning

Review limit reached

Next included review available in 38 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: d7671445-2ffb-4d95-b82f-2b7cab487377

📥 Commits

Reviewing files that changed from the base of the PR and between e6ca304 and 987667c.

⛔ Files ignored due to path filters (1)

• archify.zip is excluded by !**/*.zip

📒 Files selected for processing (19)

• archify/SKILL.md
• archify/bin/archify.mjs
• archify/bin/preview.mjs
• archify/examples/billing.erd.html
• archify/examples/billing.erd.json
• archify/recipes/scenarios.mjs
• archify/renderers/erd/render-erd.mjs
• archify/renderers/shared/cli.mjs
• archify/renderers/shared/generated-validators.mjs
• archify/renderers/shared/i18n.mjs
• archify/renderers/shared/repository-evidence.mjs
• archify/schemas/erd.schema.json
• archify/scripts/generate-validators.mjs
• archify/test/cli.test.mjs
• archify/test/erd.test.mjs
• archify/test/golden.mjs
• archify/test/guide.test.mjs
• archify/test/repository-evidence.test.mjs
• examples/billing.erd.html

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[tt-a1i]

I compared this head (987667c76fe6c9e85a597b18befe00d6bc0652d0) with #399 (fecaa6091b1e620e1263d2d1226c5bff53a5787c) against issue #115. This PR's 16 focused ERD tests pass; #399's 10 focused tests also pass. Those passes do not settle the schema contract.

Recommended consolidation: use #183 as the integration base because it already implements the issue's repository evidence, explicit cardinality requirement, standalone escape and enforcement-reference vocabulary with less shared-router churn. Reuse #399's useful typed-cardinality/marker and obstacle-routing fixtures instead of shipping two schemas. The shared architecture router is already extracted in current dev (#266), and repository evidence now has a common per-type mapping (#389); integrate those existing seams rather than restoring the old versions.

P2 reproduced blocker: changing the first billing relationship's cardinality.to to banana still makes public validate erd --json exit 0 / ok:true. The schema accepts any nonempty string and crowMarker() falls back to a one marker. Constrain cardinality to a documented, typed set and derive markers from that set; reject unrecognized values with source-specific diagnostics. Existing names such as cardinality.from/to can stay — a second field layout is not necessary to get strict semantics.

Required contract clarification: the current per-attribute primary counter rejects a two-column identity as “2 primary keys”. I reproduced this by marking a second users attribute primary. One composite identity and two unrelated primary identities are not the same assertion. Define whether this first slice represents a grouped composite identity or explicitly does not support it; do not imply that all data models with composite keys are invalid. This is a representational limit to settle before publishing schema v1, not a request for automatic database introspection.

Next owner: #183 author leads the consolidated current-dev revision, retaining authored positions and prior XML/FK/orphan fixes. Add the cardinality negative and chosen composite-identity contract tests, verify obstruction routing using the canonical router, then provide current-revision source-evidence, browser crow's-foot/attribute readability, export and isolated-package evidence plus green CI. The previous fixed defects are not being reasserted; the invalid-cardinality reproduction is new. #399 remains available for attribution and useful source/tests until a replacement is actually accepted. Neither PR is ready to merge; main remains untouched.