Skip to content

ci: pin Node 24, test every package, fix data-lit-todo id-shape assumption - #159

Merged
krisnye merged 2 commits into
mainfrom
krisnye/ci-node24-conformance-fix
Jul 28, 2026
Merged

ci: pin Node 24, test every package, fix data-lit-todo id-shape assumption#159
krisnye merged 2 commits into
mainfrom
krisnye/ci-node24-conformance-fix

Conversation

@krisnye

@krisnye krisnye commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Description

Post-merge cleanup for the persistence/sharing quadrant refactor (#158). That PR
merged green even though the data-lit-todo conformance suite was red, because
CI had two gaps. This PR fixes the cause of the failures and the CI blind
spot that let them through. No package version bump.

1. CI now tests every package (was: samples only built)

The downstream test step ran unit tests for only six packages and merely
built the private sample apps (data-lit-todo, data-lit-tictactoe, …). A
broken sample unit test could therefore merge green. The step now runs test
in every workspace package except the core (already covered by the data job)
and data-persistence (whose test pulls in Playwright — kept on its
test:node project). pnpm skips packages that declare no test script.

2. Pin Node >= 24

Set.prototype.isSupersetOf (used on the ecs readEntity path) is Node 22+,
and several suites depend on it, but nothing pinned the runtime. Pinned Node
>=24 in root engines, added/updated .nvmrc and .node-version to 24,
and bumped both CI jobs from Node 22 to 24.

3. Fix data-lit-todo conformance id-shape assumption

The conformance harness assumed the ecs entity id equalled the spec's domain
id (1..N in display order). That identity held only under the old
sign-based id scheme; the quadrant encoding gives document entities stride-4
ids (0, 4, 8, …), so every id-addressed case broke.

Made the harness id-shape-agnostic rather than re-coupling it to the new
encoding or adding a domain-id component:

  • fromState returns the seeded entities in display order; the runner maps
    spec id → entity positionally and passes a resolve into each apply, so
    id-addressed transactions (delete / toggle / drag) target the right entity.
  • The projection halves compare with expectStateMatchesIgnoringIds, which
    canonicalises todo ids before comparison — behaviour is checked on the
    visible fields, never on the id value. The pure-spec half stays id-strict
    (the spec fully owns its domain ids).

The app model is unchanged; this touches test-only conformance code.

Verification

On Node 24, full CI-equivalent suite is green:

  • @adobe/data: typecheck + lint + test:ci — 2921 tests pass.
  • All downstream packages test (incl. every sample app) pass; data-lit-todo
    79/79.
  • @adobe/data-persistence test:node — 139 pass.

Related PRs

🤖 Generated with Claude Code

krisnye and others added 2 commits July 28, 2026 12:51
…ption

CI let a broken data-lit-todo conformance suite merge green (#158) for two
reasons, both fixed here:

1. The downstream test step only ran six packages and *built* — never
   tested — the private sample apps. Broadened the filter to run `test` in
   every workspace package except the core (covered by the `data` job) and
   data-persistence (Playwright — kept on its `test:node` project). This now
   includes data-lit-todo and the other samples.

2. `Set.prototype.isSupersetOf` (used in the ecs readEntity path) is Node 22+
   and several suites need it; CI ran Node 22 but nothing pinned it. Pinned
   Node >=24 (engines, .nvmrc, .node-version) and bumped both CI jobs to 24.

The data-lit-todo conformance harness assumed ecs entity ids equalled the
spec's domain ids (1..N) — true only under the old sign-based id scheme, broken
by the persistence/sharing quadrant encoding (stride-4 ids). Made the harness
id-shape-agnostic instead: fromState returns the seeded entities so the runner
maps spec id -> entity, and the projection halves compare ignoring the todo id.
No domain-id component added; the app is unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the hand-rolled toEntity(1_000_000, 0) in the conformance harness with
a first-class Entity.none: the last representable id (max local index in the
last quadrant, numerically -1). The allocator issues local indices from 0 and a
quadrant fills long before 2^30 entities, so it never collides with a live
entity and locate/read resolve it to null. The bit layout stays owned by
persistence-sharing.ts.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@krisnye
krisnye merged commit 2cc567c into main Jul 28, 2026
4 checks passed
@krisnye
krisnye deleted the krisnye/ci-node24-conformance-fix branch July 28, 2026 20:26
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.

1 participant