fix(data): preserve historical metadata integrity#194
Conversation
📝 WalkthroughWalkthrough本次变更调整属性差异和版本比较规则，限制元数据缓存容量，并收紧组件 major 快照回填范围；测试覆盖预发布版本排序、缓存淘汰、属性移除和缓存快照隔离行为。 Changes核心数据行为
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
📦 Package Size Report
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #194 +/- ##
==========================================
- Coverage 99.69% 99.69% -0.01%
==========================================
Files 40 40
Lines 2603 2598 -5
Branches 803 800 -3
==========================================
- Hits 2595 2590 -5
Misses 8 8 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the metadata loading and changelog generation logic. Key changes include limiting the metadata cache to 32 entries with an eviction policy, removing major-version API data backfilling (restricting backfills to descriptions), updating semver comparison to correctly handle prerelease identifiers, and removing speculative rename guessing for removed properties. The reviewer feedback highlights three main improvement opportunities: cleaning up dead code (replacement property and printing logic) left over from removing speculative renames, replacing the while loop with an if statement in the cache eviction logic for better readability, and performing a deeper clone of storedComp in resolveComponent to prevent nested arrays and objects from mutating the shared cache.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| (p) => p.type === prop.type && !oldMap.has(p.name), | ||
| ); | ||
| removed.push({ name, type: prop.type, replacement: possibleRename?.name }); | ||
| removed.push({ name, type: prop.type }); |
There was a problem hiding this comment.
With the removal of the speculative rename inference, the replacement property is no longer populated in the removed prop diffs. However, the PropDiff interface still defines replacement?: string; and printApiDiff still contains logic to print the replacement note (lines 286-288). This results in dead code.\n\nConsider removing replacement from the PropDiff interface and cleaning up the unused printing logic in printApiDiff to keep the codebase clean and maintainable.
| while (metadataCache.size > MAX_METADATA_CACHE_ENTRIES) { | ||
| const oldestKey = metadataCache.keys().next().value; | ||
| if (oldestKey === undefined) break; | ||
| metadataCache.delete(oldestKey); | ||
| } |
There was a problem hiding this comment.
Since metadataCache.set is only called when the key does not already exist in the cache, the cache size can only exceed MAX_METADATA_CACHE_ENTRIES by at most 1. Using an if statement instead of a while loop is more idiomatic, improves readability, and avoids any theoretical risk of an infinite loop.
| while (metadataCache.size > MAX_METADATA_CACHE_ENTRIES) { | |
| const oldestKey = metadataCache.keys().next().value; | |
| if (oldestKey === undefined) break; | |
| metadataCache.delete(oldestKey); | |
| } | |
| if (metadataCache.size > MAX_METADATA_CACHE_ENTRIES) { | |
| const oldestKey = metadataCache.keys().next().value; | |
| if (oldestKey !== undefined) { | |
| metadataCache.delete(oldestKey); | |
| } | |
| } |
| suggestion ? `Did you mean '${suggestion}'?` : undefined, | ||
| ); | ||
| } | ||
| const comp = { ...storedComp }; |
There was a problem hiding this comment.
The shallow copy { ...storedComp } only copies the top-level properties of the component. Nested arrays and objects, such as props and subComponentProps, still share the same references as the cached storedComp. If a caller of resolveComponent mutates these arrays or objects (e.g., sorting, filtering, or modifying elements), it will pollute the shared metadata cache.\n\nTo ensure true immutability of the cached metadata, consider cloning the props array and subComponentProps object as well.
const comp = {
...storedComp,
props: [...storedComp.props],
subComponentProps: storedComp.subComponentProps
? Object.fromEntries(
Object.entries(storedComp.subComponentProps).map(([key, val]) => [key, [...val]]),
)
: undefined,
};1 participant
Summary
Verification
npm run buildnpm run typechecknpm audit --json— 0 vulnerabilities on the base dependency setgit diff --checkThe default parallel full suite was also run; two existing
lint --difftests intermittently exceeded the 5-second test timeout under load. Both exact tests pass in isolation, and no lint files are changed in this PR.