Synced from GitHub
test(web): attachment preview omission tests no longer time out on CI#25514
The "keeps the body and warns when omitting expanded %s" cases in AttachmentFilePreview.browser.test.tsx hit vitest's 15s timeout on CI for PRs that don't touch this area: run 40332078618 failed comments, styles and style chain, and run 40334346059 failed styles. In the failing run every one of the 12 cases was slow, taking 3.6–15.5s. Other full-UI Word render tests in the same file took about 1.5s.
Cause. The shared fixture body had 500 paragraphs, each with a page break and note references. The expansion budget needs only about 13 copies of the 8,000-unit payload to exceed 100,000 markup units. The style and numbering cases are cut by their own part budgets and don't depend on the body at all. So most of the fixture did nothing for the assertion. It only made every case lay out and mount a 501-page document while the test waited, and on a busy runner that work grew enough to cross the timeout.
Fix. The body now has 20 page-break paragraphs: 160,000 units, still 60% over budget. Every case still asserts the same things: the body is kept, the "Preview limited" notice appears, there's no alert, and the omitted payload isn't rendered. No timeouts, retries or sleeps were added.
Timings. Local, with vitest and Chromium pinned to 2 cores next to 2 busy-loop processes (a rough stand-in for a 2-vCPU runner):
| case | before | after |
|---|---|---|
| footnotes | 7218ms | 6002ms |
| note roots | 5532ms | 5375ms |
| endnotes | 5670ms | 4179ms |
| header | 5074ms | 4020ms |
| footer | 5263ms | 4220ms |
| comments | 5060ms | 4163ms |
| styles | 5318ms | 3681ms |
| style chain | 6261ms | 4331ms |
| style count | 6415ms | 4098ms |
| style visits | 5087ms | 5424ms |
| numbering | 5805ms | 4463ms |
| picture numbering | 5469ms | 3888ms |
| total (12 cases) | 98.3s | 83.1s |
After the fix, these cases cost about the same as the "retains Word … without a limit warning" render tests under the same load (3.5–6.2s), and those take about 1.5s on CI. What remains is the fixed cost of starting the layout worker (yrs WASM session and fonts), which every Word render test pays. Without load the 12 cases take 1.4–2.9s.
Opus 5.5 via Claude Code.
🤖 Generated with Claude Code
Checks
23 successful · 12 skipped
Added 1 commit
- test(web): attachment preview omission tests no longer time out on CI65172a0
Synced from GitHub
Approvability
Verdict: Approved at 65172a0
Macroscope's review found this PR approvable — This is a five-line change confined to an ignored browser test fixture, intended to reduce CI timeout risk without changing product behavior. Its impact is limited to test execution and does not affect production defaults or static-analysis configuration.
Notes:
- No code objects were reviewed. Approvability was decided on eligibility alone.
You can add or adjust custom eligibility rules. Learn more.
Synced from GitHub
No actionable comments were generated in the recent review.
External image Recent review info
-
External image Run configuration
- Source mode: Auto
- Repository: pingdotgg/t3code
- Branch: fix/attachment-preview-flake
- File: .coderabbit.config.ts
- Commit: 65172a0
- Layers: Repository TS config
- Review profile: CHILL
- Plan: Team
- Run ID:
f14c712a-699b-458a-bad3-7aff14381226
External image Commits
Reviewing files that changed from the base of the PR and between f05c38f1667fcf36970affd09574a234c85e3bff and 65172a0f05b3d5dd19f6738345c72be534d95770.
External image Files selected for processing: 1
apps/web/src/components/files/AttachmentFilePreview.browser.test.tsx
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
Walkthrough
The Word expansion test now uses 20 repeated paragraphs instead of 500. Its comments describe the 100,000-unit expansion budget and the layout time added by more pages.
Changes: Word expansion test
Layer / File(s) Summary Update Word expansion budget test
apps/web/src/components/files/AttachmentFilePreview.browser.test.tsxThe test creates 20 repeated paragraphs. Its comments state that 20 references or pages exceed the 100,000-unit expansion budget and add layout time. Priority: External image Low
Estimated code review effort: 1 (Trivial) | ~5 minutes
Change: Other
Suggested reviewers:
maria-rcks
Merge Risk: External image Minimal · up to 65172
The test remains effective with a substantially smaller fixture and introduces no merge-blocking risk.
Pre-merge checks: Passed 4
Check name Status Explanation Title check Passed The title clearly states that the change prevents attachment preview omission tests from timing out on CI. It uses a concise conventional commit format. Description check Passed The description clearly explains the CI timeout problem, the fixture-size change, preserved assertions, and verification timings. It does not explicitly document issue or maintainer approval, but the … Linked Issues check Passed Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request. Full details: Description check
Explanation
The description clearly explains the CI timeout problem, the fixture-size change, preserved assertions, and verification timings. It does not explicitly document issue or maintainer approval, but the remaining content is complete for this focused test-only fix.
Finishing Touches
📝 Generate docstrings
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
- Commit to this branch
- Create a new PR
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.