Keep large Pierre diffs responsive - #136
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f69a2c2754
ℹ️ 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".
| id: codeViewItemId, | ||
| type: 'diff', | ||
| fileDiff: getSingularPatch(patch), | ||
| annotations: lineAnnotations, |
There was a problem hiding this comment.
Bump CodeView item identity when patches change
When a user switches between two already-loaded large files that both stay on the CodeView path, this publishes the new fileDiff and annotations under the same item id with no version. Pierre CodeView reconciles controlled items by id and only replaces the stored snapshot when the item version changes, so the second file can continue showing the previous large diff/comments while the surrounding file selector and review state have moved on. Use a path/content-derived id or bump version whenever patch or annotations change.
Useful? React with 👍 / 👎.
| textScale = 1.12, | ||
| ) { | ||
| const { stickyHeader, ...options } = baseDiffOptions; | ||
| const lineHeight = 10.5 * 1.55 * textScale; |
There was a problem hiding this comment.
Pass the active text scale into CodeView metrics
In dashboards configured with compact, large, or a custom appearance.textScale, the CodeView path still estimates row height with the hard-coded 1.12 default while unsafeCSS renders rows from the inherited --deck-text-scale. Because CodeView uses itemMetrics to compute virtualized heights, scroll targets, and selection/anchor positions, non-default densities can mis-size large diffs even though the old PatchDiff path scaled directly with CSS. Thread the resolved appearance scale into this option or derive the metric from the same source as the CSS variable.
Useful? React with 👍 / 👎.
Summary
Why
The retained 5,000- and 10,000-row baselines mounted 20,099 and 40,099 DOM nodes and accumulated 210 ms and 405 ms of long tasks. Direct CodeView was slower for an ordinary patch, so this uses a measured threshold instead of applying virtualization universally.
Measurement
Verification
npm run check(84 files, 524 tests)npm run format:checkgit diff --check