feat: recover provenance past the per-object recording budget - #62
Conversation
#registerOrigin walks a response into #valueOrigins but stops after 100 entries per object. A list element past the hundredth was therefore never registered, and since a map callback hands the element itself to JSX with no path back to the response, the lookup found nothing at all. That is the worst shape a failure can take here: silent and dependent on position. Two rows that look identical differed in whether they could be explained, with nothing said about why. Raising the constant only moves the cliff, so registration stays bounded and the recovery moves to lookup time. A container that was cut off is remembered by WeakRef, and a lookup that misses scans those containers for the value by identity, building the path from the container's own origin. Registration stays O(1) for the tail; the scan runs only when someone inspects an element, where it is affordable. Reads enumerable data properties only, through a helper now shared with registration: resolving a lookup must never invoke an application getter. Covered by a test that fails if it ever does. Raises the npm tarball budget from 128 KiB to 130 KiB. It has held since it was introduced and should keep holding, so the raise is deliberate and stated rather than quietly absorbed: 127.3 KiB left 0.7 KiB of headroom and this costs 1.6 KiB. Verified the three positive tests fail with the recovery disabled. Co-Authored-By: Claude Opus 5 <[email protected]>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1be8d8908e
ℹ️ 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".
| const found = Array.isArray(container) | ||
| ? container.indexOf(value) | ||
| : dataEntries(container).find(([, child]) => child === value)?.[0]; |
There was a problem hiding this comment.
When a remembered array or record is mutated after registerValueOrigin—for example, by pushing a locally created row—this lookup scans the container's current contents and labels that new object as a confirmed child of the earlier network/store origin. Because membership at lookup time is not evidence that the value existed when the origin was registered, recovery must either preserve registration-time membership or report the result as possible/unavailable rather than manufacturing a confirmed path.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| return [{ | ||
| ...entry.origin, | ||
| id: createId("cs_origin", ++this.#sequence), | ||
| path: `${entry.origin.path ?? "value"}${this.#containerSegment(container, key)}`, | ||
| }]; |
There was a problem hiding this comment.
When the same tail object is present in two remembered containers with different origins, this immediate return chooses only the newest match and reports it as confirmed, even though object identity cannot establish which container supplied the rendered value. The recovery should collect all matching origins or explicitly downgrade/withhold the attribution instead of silently selecting one causal chain.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| const found = Array.isArray(container) | ||
| ? container.indexOf(value) | ||
| : dataEntries(container).find(([, child]) => child === value)?.[0]; |
There was a problem hiding this comment.
For a truncated array containing an accessor-backed index, indexOf performs ordinary property reads and invokes that accessor while searching for a later item or a missing value. This defeats the new data-property-only safety guarantee and can run or mutate application code merely because the user inspected an element; arrays should be searched through descriptors just like non-array containers.
Useful? React with 👍 / 👎.
| : []; | ||
| if ((typeof value === "object" && value !== null) || typeof value === "function") { | ||
| origins.push(...(this.#valueOrigins.get(value as object) ?? [])); | ||
| origins.push(...this.#originsForObject(value as object)); |
There was a problem hiding this comment.
When an instrumented render maps a large registered list, every row past the first 100 reaches this fallback during traceDerived capture, rather than when a person later inspects the row. Each tail object is found with an indexOf scan from the beginning of the container, so rendering an n-row list performs quadratic recovery work; the cited 10,000-row case can therefore block the application during rendering. Defer this scan until actual inspection or use a lookup strategy whose render-time work remains bounded.
AGENTS.md reference: AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
1 participant
Third and last gap from the chain audit that started with the r/reactjs comment. Found while probing the previous PR's claim — and unlike the item I dropped in #61, this one is real.
The gap
#registerOriginwalks a response into#valueOriginsbut stops after 100 entries per object. A.map()callback hands the element itself to JSX, so there is no path back to the response — only the object. If that object was never registered, the lookup finds nothing.Measured, not assumed:
A 200-row list explains its first 100 rows and silently refuses the rest. That is the worst shape a failure can take in a tool whose premise is never misleading: silent and position-dependent. Two rows that look identical differ in whether they can be explained, and nothing says why.
Change
Raising the constant only moves the cliff — a 10k-row table breaks just the same — so registration stays bounded and the recovery moves to lookup time.
WeakRef, bounded to the last 32.Budget
This raises the npm tarball budget from 128 KiB to 130 KiB, and that deserves to be called out rather than absorbed quietly.
mainThe budget has held since it was introduced in #23 and should keep holding — the point is to catch growth nobody decided on. I tried trimming first (extracting the shared helper) and it came out 0.1 KiB worse, since the tarball carries source maps and declarations. So the trade is stated instead: 1.6 KiB for removing a silent, position-dependent failure. The constant now carries a comment explaining when it moved and why.
If you would rather not spend it, the alternative is dropping the non-array branch — cheaper, but it leaves keyed records over 100 keys broken, which is a known gap rather than a fixed one.
Verification
pnpm typecheck,pnpm test: 24/24 tasks.verify:release,verify:public-api(5 entrypoints, snapshot unchanged),verify:trace-contract: pass.🤖 Generated with Claude Code