Skip to content

fix(endpoint): Show a re-added item once in a Collection - #4487

Open
ntucker wants to merge 13 commits into
masterfrom
claude/collection-push-readd
Open

ntucker wants to merge 13 commits into
masterfrom
claude/collection-push-readd

Conversation

@ntucker

@ntucker ntucker commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Requested by Nathaniel · project thread

Motivation

Bug fix. A list is keyed by another entity's id, for example cart lines keyed by product, or followed users. The app deletes an item, then creates an item with the same id and adds it through push or unshift.

Instance: in the computed-properties cart demo (#4484), the reader removes the mug's last unit (CartResource.delete), then adds the mug again (getList.push). The cart list shows the mug twice. A Query over getList.schema counts 5 items instead of 4. Tracked as Linear REACT-13.

Solution

  • Cause: delete (an Invalidate schema) marks the entity INVALID and leaves its id in every Collection that held it. Denormalize skips INVALID entities, so the stale id stays hidden until the same pk is created again. Then both the stale id and the pushed one show, so from the user's side push added two items.
  • Fix: before an Array Collection create (push, unshift, addWith) normalizes its input, it checks each incoming item's pk. If that entity is currently deleted, the create drops that id from the matching collections as part of the merge, then adds as usual. This check has to happen before normalizing, because normalizing revives the entity.
  • push and unshift keep Array.push()/unshift() semantics: nothing is deduped or moved, and pushing an id the list already shows (not deleted) still adds it again.
  • The drop happens inside the collection's merge (merge/mergeWithStore see the list without the dropped ids), so the list's meta and shouldReorder behave as before.
  • Cost: one process/pk/getEntity per incoming item. Only when an incoming item was deleted does the merge filter the list. Ids passed as refs are skipped, since normalize passes them through without processing.
await ctrl.fetch(CartResource.delete, { id: 'mug' });
await ctrl.fetch(CartResource.getList.push, { id: 'mug', quantity: 1 });
// getList shows the mug once, at the end

Assumptions and validation

  • High: the new tests fail on master and pass here. They cover a deleted then re-created id for push, unshift and a Union collection holding null, and a collection the create doesn't target staying untouched. A live duplicate is still added (Array semantics), list meta is kept for an out-of-order create, and ids passed as refs aren't processed. All package suites pass locally (1937 tests), and tsc is clean.

  • High, V8 check (.cursor/rules/v8-optimization.mdc, Haiku, examples/benchmark spread suite, "collection push 10 onto 10k", 8 interleaved runs per ref):

    master this PR
    ops/sec 116.4 ± 3.4 115.2 ± 3.2 (-1.0%, within noise)
    deopts in normalizeCreate / deletedRefs 0 0
    P/N ICs for key / schema / process / isSingleSchema 16 / 8 / 5 / 2 16 / 8 / 5 / 2

    An earlier version that scanned every member of each collection measured -7.9%, which is why only the incoming items are checked now. The benchmark has no deleted entities, so the drop path (withoutRefs, added after this run) isn't measured. It runs only when an incoming item was deleted, and it doesn't change the no-deletion path.

  • Medium: isShallowEqual gains a null guard, because removeMerge now also runs against Union lists that hold null.

  • Not covered: Immutable.js state that stores a collection as a List keeps the old behavior (the drop only applies to arrays).


Generated by Claude Code

🤖 Generated with Claude Code

https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5


Note

Medium Risk
Touches core Collection normalization/merge for all push/unshift creates; behavior change is scoped to deleted-entity re-adds but affects shared cache state for nested lists.

Overview
Fixes a duplicate list entry when an entity is invalidated (deleted) and later re-added via Collection push or unshift (e.g. getList.push). Invalidate leaves the old id in the stored collection while denormalize hides it; re-creating the same pk then merged both the stale ref and the new one.

normalizeCreate now scans incoming items before normalize (so entities are not revived yet), finds pks whose store entry is INVALID, and merges through a withoutRefs wrapper that strips those refs from the existing list before the usual push/unshift merge. isShallowEqual gains null checks so removal works on Union lists that contain null. Normal Array.push/unshift semantics are unchanged: live duplicates are still appended, and only targeted collections are updated.

Adds regression tests (including Union members) and a patch changeset; the v0.19 blog notes the fix.

Reviewed by Cursor Bugbot for commit d140e03. Bugbot is set up for automated code reviews on this repo. Configure here.

Deleting an item invalidates its entity but leaves its id in each Collection
that held it. Pushing (or unshifting) an item with the same id then appended a
second copy of the id, so the list showed the item twice while All() counted it
once. push and unshift now drop an id already in the list before adding it, so
the item moves to the end or start.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5
@changeset-bot

changeset-bot Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: d140e03

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 10 packages
Name Type
@data-client/endpoint Patch
@data-client/graphql Patch
@data-client/rest Patch
example-benchmark Patch
example-benchmark-react Patch
normalizr-github-example Patch
normalizr-redux-example Patch
normalizr-relationships Patch
test-bundlesize Patch
coinbase-lite Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-27T01:49:57.841078Z
We couldn’t complete this review. Comment @codex review to try again.
d140e03 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

ntucker pushed a commit that referenced this pull request Oct 27, 2026
The Query now reads the same list the cart page renders, and only its
new Query line is highlighted. Remove-then-add counts correctly with #4487.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 05d723f54f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/endpoint/src/schemas/Collection.ts Outdated
@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Size Change: +186 B (+0.17%)

Total Size: 112 kB

📦 View Changed
Filename Size Change
examples/test-bundlesize/dist/rdcEndpoint.js 8.23 kB +186 B (+2.31%)
ℹ️ View Unchanged
Filename Size
examples/test-bundlesize/dist/App.js 1.45 kB
examples/test-bundlesize/dist/polyfill.js 301 B
examples/test-bundlesize/dist/rdcClient.js 10.8 kB
examples/test-bundlesize/dist/rdcNextjs.js 12.4 kB
examples/test-bundlesize/dist/rdcPipeableStream.js 9.65 kB
examples/test-bundlesize/dist/react.js 68.6 kB
examples/test-bundlesize/dist/webpack-runtime.js 698 B

compressed-size-action

@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Docs preview: https://docs-site-pr-4487-data-client.vercel.app

Built from d140e03 merged into master (this build). Locales: en (the merge queue builds every locale).

@github-actions github-actions Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Benchmark React

Details
Benchmark suite Current: d140e03 Previous: 8817f47 Ratio
data-client: getlist-100 146.18 ops/s (± 7.7%) 222.22 ops/s (± 8.5%) 1.52
data-client: getlist-500 51.28 ops/s (± 4.9%) 73.27 ops/s (± 6.4%) 1.43
data-client: update-entity 312.5 ops/s (± 7.2%) 666.67 ops/s (± 6.1%) 2.13
data-client: update-user 333.33 ops/s (± 7.8%) 625 ops/s (± 6.8%) 1.88
data-client: getlist-500-sorted 55.87 ops/s (± 8.6%) 64.78 ops/s (± 9.3%) 1.16
data-client: update-entity-sorted 277.78 ops/s (± 7.4%) 444.66 ops/s (± 10.7%) 1.60
data-client: update-entity-multi-view 312.5 ops/s (± 6.2%) 555.56 ops/s (± 7.0%) 1.78
data-client: list-detail-switch-10 10.27 ops/s (± 7.6%) 20.64 ops/s (± 8.0%) 2.01
data-client: update-user-10000 81.3 ops/s (± 8.6%) 125.79 ops/s (± 11.2%) 1.55
data-client: invalidate-and-resolve 40.98 ops/s (± 5.2%) 60.61 ops/s (± 7.1%) 1.48
data-client: unshift-item 250 ops/s (± 1.5%) 384.62 ops/s (± 0.0%) 1.54
data-client: delete-item 222.22 ops/s (± 12.6%) 555.56 ops/s (± 9.1%) 2.50
data-client: move-item 166.67 ops/s (± 6.0%) 307.77 ops/s (± 6.0%) 1.85

This comment was automatically generated by workflow using github-action-benchmark.

@codecov

codecov Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 98.41%. Comparing base (6394016) to head (d140e03).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
packages/endpoint/src/schemas/Collection.ts 96.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4487      +/-   ##
==========================================
+ Coverage   98.37%   98.41%   +0.04%     
==========================================
  Files         161      161              
  Lines        3080     3101      +21     
  Branches      605      612       +7     
==========================================
+ Hits         3030     3052      +22     
  Misses         10       10              
+ Partials       40       39       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Benchmark

Details
Benchmark suite Current: d140e03 Previous: 7adefee Ratio
normalizeLong 435 ops/sec (±3.85%) 693 ops/sec (±3.56%) 1.59
normalizeLong Values 397 ops/sec (±1.06%) 607 ops/sec (±1.11%) 1.53
normalizeLong Scalar 370 ops/sec (±3.66%) 544 ops/sec (±2.39%) 1.47
normalizeLong Scalar update 914 ops/sec (±0.80%) 1574 ops/sec (±1.86%) 1.72
denormalizeLong 246 ops/sec (±6.26%) 349 ops/sec (±5.61%) 1.42
denormalizeLong Values 239 ops/sec (±4.60%) 322 ops/sec (±3.99%) 1.35
denormalizeLong donotcache 985 ops/sec (±1.06%) 1820 ops/sec (±2.03%) 1.85
denormalizeLong Values donotcache 745 ops/sec (±0.12%) 1151 ops/sec (±5.18%) 1.54
denormalizeLong Scalar donotcache 1052 ops/sec (±0.28%) 1909 ops/sec (±0.48%) 1.81
denormalizeShort donotcache 500x 1418 ops/sec (±0.14%) 2462 ops/sec (±1.91%) 1.74
denormalizeShort 500x 678 ops/sec (±6.93%) 899 ops/sec (±7.30%) 1.33
denormalizeShort 500x withCache 6961 ops/sec (±0.08%) 9181 ops/sec (±2.20%) 1.32
queryShort 500x withCache 3262 ops/sec (±0.07%) 5359 ops/sec (±0.74%) 1.64
buildQueryKey All 59263 ops/sec (±0.94%) 98489 ops/sec (±0.41%) 1.66
query All withCache 6212 ops/sec (±2.62%) 10256 ops/sec (±5.77%) 1.65
denormalizeLong with mixin Entity 225 ops/sec (±7.29%) 335 ops/sec (±6.11%) 1.49
denormalizeLong withCache 6914 ops/sec (±0.20%) 12871 ops/sec (±0.43%) 1.86
denormalizeLong withCache (Scalar churn) 6851 ops/sec (±1.01%) 12670 ops/sec (±1.00%) 1.85
denormalizeLong Values withCache 5125 ops/sec (±1.60%) 9804 ops/sec (±1.03%) 1.91
denormalizeLong Scalar withCache 7890 ops/sec (±0.22%) 13747 ops/sec (±0.99%) 1.74
denormalizeLong Scalar update withCache 4156 ops/sec (±0.42%) 7523 ops/sec (±0.38%) 1.81
denormalizeLong All withCache 6370 ops/sec (±0.29%) 13111 ops/sec (±0.23%) 2.06
denormalizeLong Query-sorted withCache 6498 ops/sec (±2.51%) 10436 ops/sec (±6.87%) 1.61
denormalizeLongAndShort withEntityCacheOnly 1770 ops/sec (±0.22%) 3035 ops/sec (±1.13%) 1.71
denormalize bidirectional 50 4752 ops/sec (±10.75%) 7229 ops/sec (±9.22%) 1.52
denormalize bidirectional 50 donotcache 41461 ops/sec (±0.55%) 74415 ops/sec (±0.62%) 1.79
getResponse 4478 ops/sec (±4.14%) 6850 ops/sec (±6.61%) 1.53
getResponse (null) 10023771 ops/sec (±0.98%) 19028450 ops/sec (±3.52%) 1.90
getResponse (clear cache) 215 ops/sec (±7.50%) 333 ops/sec (±7.84%) 1.55
getSmallResponse 3568 ops/sec (±0.94%) 6392 ops/sec (±0.50%) 1.79
getSmallInferredResponse 2857 ops/sec (±0.09%) 4838 ops/sec (±0.54%) 1.69
getResponse Collection 4663 ops/sec (±2.00%) 8037 ops/sec (±2.20%) 1.72
get Collection 3427 ops/sec (±0.18%) 6524 ops/sec (±0.49%) 1.90
get Query-sorted 4972 ops/sec (±1.75%) 7759 ops/sec (±5.02%) 1.56
setLong 437 ops/sec (±1.31%) 737 ops/sec (±0.35%) 1.69
setLongWithMerge 247 ops/sec (±0.71%) 415 ops/sec (±1.86%) 1.68
setLongWithSimpleMerge 264 ops/sec (±0.13%) 443 ops/sec (±1.35%) 1.68
setSmallResponse 500x 935 ops/sec (±0.77%) 1611 ops/sec (±1.41%) 1.72
setMany 50x one-per-row 160 ops/sec (±0.34%) 220 ops/sec (±1.51%) 1.38
setMany 50 batch 3702 ops/sec (±0.44%) 6279 ops/sec (±0.55%) 1.70
setMany 500x one-per-row 16.37 ops/sec (±0.51%) 22.67 ops/sec (±0.44%) 1.38
setMany 500 batch 1441 ops/sec (±0.18%) 2821 ops/sec (±0.35%) 1.96

This comment was automatically generated by workflow using github-action-benchmark.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6add6a52c6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/endpoint/src/schemas/Collection.ts Outdated
push and unshift append like Array.push() again. Before adding, a create
drops refs to deleted (INVALID) entities from the matching collections, so
an item re-created with a deleted pk shows once.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5
Checks each incoming item's pk before normalizing it, so a create into a
large collection no longer scans every member when nothing was deleted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 22c9d55c02

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/endpoint/src/schemas/Collection.ts Outdated
Comment thread packages/endpoint/src/schemas/Collection.ts
…leted refs

Drop the deleted refs inside the merge, so the stored list keeps its meta
and out-of-order responses still reorder. Ids passed as refs aren't
processed, since normalize passes them through.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5
Record which incoming items were deleted, then drop the refs normalize
produced for them, so the ref format lives in one place. Hoist the test
meta fixture and drop a test the Union deletion test already covers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

State that ArraySchema.normalize keeps one ref per input item, and test a
push where a deleted item follows a live one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

Build the repeated todos lists and their meta with one helper, and drop a
call-site comment the helper's docblock already covers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015gC38VCqJFH8PMKWL49Le5
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

This branch was successfully deployed

1 active deployment
docs-preview — d140e03b Deployed Oct 27, 2026 by ntucker via preview / docs deploy #4233
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