fix: add decode worker heartbreak message#2599
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds a ChangesDecode Worker Heartbreak Messaging
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
🦋 Changeset detectedLatest commit: b349c68 The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
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 |
PupilTong
force-pushed
the
codex/decode-worker-heartbreak
branch
from
ec69f2a to
f083b51
Compare
PupilTong
changed the title
PupilTong
force-pushed
the
codex/decode-worker-heartbreak
branch
from
f083b51 to
04eaac0
Compare
PupilTong
force-pushed
the
codex/decode-worker-heartbreak
branch
from
04eaac0 to
120e984
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/decode-worker-heartbreak.md:
- Line 5: The changeset note text "Add decode worker heartbreak messages during
template loads." is inaccurate because the worker now emits `heartbreak`
periodically for its entire lifetime; update the changeset note string to
reflect that (e.g., mention periodic/continuous emission or "for its lifetime"
and the `heartbreak` event name) so the message accurately describes runtime
behavior; locate and edit the changeset note content (the current note string)
to a wording like "Emit decode worker `heartbreak` messages periodically for the
worker's lifetime" or a similar concise phrase.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 644c63f8-ae5e-4931-bf70-ca3963518191
📒 Files selected for processing (5)
.changeset/decode-worker-heartbreak.mdpackages/web-platform/web-core/tests/template-manager.spec.tspackages/web-platform/web-core/ts/client/decodeWorker/decode.worker.tspackages/web-platform/web-core/ts/client/decodeWorker/types.tspackages/web-platform/web-core/ts/client/mainthread/TemplateManager.ts
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/web-platform/web-core/tests/template-manager.spec.ts (2)
58-58: 💤 Low valueTest name implies bidirectional exchange but only verifies emission.
The test name "should exchange worker-level heartbreak messages" suggests both sending and receiving/handling are verified. However, the test only confirms that heartbreak messages are posted via
globalThis.postMessage. It doesn't verify thatTemplateManagerreceives, processes, or replies to these messages (though the PR summary mentions TemplateManager ignores them).Consider renaming to something like "should emit periodic worker-level heartbreak messages" for accuracy, or extend the test to verify TemplateManager's handling if that's important.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web-platform/web-core/tests/template-manager.spec.ts` at line 58, The test name claims bidirectional "exchange" but only asserts emission via globalThis.postMessage; rename the test string in the test case (currently "should exchange worker-level heartbreak messages") to a precise name such as "should emit periodic worker-level heartbreak messages". Locate the test declaration in template-manager.spec.ts and update the title passed to test(...) accordingly; alternatively if you prefer to verify TemplateManager handling instead, add assertions that TemplateManager's message handler receives/ignores the heartbreak messages by spying/mocking the TemplateManager message callback and asserting its behavior after emission of globalThis.postMessage.
63-64: 💤 Low valueTighten type checking using actual message types.
The type assertion
(message as { type?: string }).type === 'heartbreak'is loose. Since the codebase defines message types inpackages/web-platform/web-core/ts/client/decodeWorker/types.ts(per PR summary), consider importing and using the actualHeartbreakMessageorWorkerMessagetype for better type safety and autocomplete.♻️ Example improvement
+import type { WorkerMessage } from '../ts/client/decodeWorker/types.js'; + test('should exchange worker-level heartbreak messages', async () => { const postMessageSpy = vi.spyOn(globalThis, 'postMessage'); try { await new Promise(resolve => setTimeout(resolve, 2200)); const heartbreakMessages = postMessageSpy.mock.calls.filter( - ([message]) => (message as { type?: string }).type === 'heartbreak', + ([message]) => (message as WorkerMessage).type === 'heartbreak', ); expect(heartbreakMessages.length).toBeGreaterThanOrEqual(2);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web-platform/web-core/tests/template-manager.spec.ts` around lines 63 - 64, Replace the loose type assertion in the filter for heartbreakMessages with the actual worker message types: import the WorkerMessage/HeartbreakMessage type from the project's types file and use a type guard (or a typed predicate) in the filter so TypeScript knows the filtered entries are HeartbreakMessage; update the filter on postMessageSpy.mock.calls (and any subsequent uses of those entries) to rely on that type guard instead of (message as { type?: string }).type === 'heartbreak' so you get proper type safety and autocomplete for HeartbreakMessage fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/web-platform/web-core/tests/template-manager.spec.ts`:
- Around line 58-70: The test "should exchange worker-level heartbreak messages"
uses a fixed 2200ms sleep which is flaky; change it to poll until at least two
'heartbreak' messages are observed or a generous timeout elapses instead of a
hard sleep: use the existing postMessageSpy (vi.spyOn(globalThis,
'postMessage')) and repeatedly check postMessageSpy.mock.calls.filter(([m]) =>
(m as {type?:string}).type === 'heartbreak') in a short loop (e.g., await small
delays) until the count >= 2 or a max wait (e.g., 5s) is reached, then assert
length >= 2 and restore the spy in finally; this makes the test resilient to
slow CI and heartbeat-interval changes while keeping the same assertions.
---
Nitpick comments:
In `@packages/web-platform/web-core/tests/template-manager.spec.ts`:
- Line 58: The test name claims bidirectional "exchange" but only asserts
emission via globalThis.postMessage; rename the test string in the test case
(currently "should exchange worker-level heartbreak messages") to a precise name
such as "should emit periodic worker-level heartbreak messages". Locate the test
declaration in template-manager.spec.ts and update the title passed to test(...)
accordingly; alternatively if you prefer to verify TemplateManager handling
instead, add assertions that TemplateManager's message handler receives/ignores
the heartbreak messages by spying/mocking the TemplateManager message callback
and asserting its behavior after emission of globalThis.postMessage.
- Around line 63-64: Replace the loose type assertion in the filter for
heartbreakMessages with the actual worker message types: import the
WorkerMessage/HeartbreakMessage type from the project's types file and use a
type guard (or a typed predicate) in the filter so TypeScript knows the filtered
entries are HeartbreakMessage; update the filter on postMessageSpy.mock.calls
(and any subsequent uses of those entries) to rely on that type guard instead of
(message as { type?: string }).type === 'heartbreak' so you get proper type
safety and autocomplete for HeartbreakMessage fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d79282c1-8860-4c6a-b6c1-f9a6045cc576
📒 Files selected for processing (5)
.changeset/decode-worker-heartbreak.mdpackages/web-platform/web-core/tests/template-manager.spec.tspackages/web-platform/web-core/ts/client/decodeWorker/decode.worker.tspackages/web-platform/web-core/ts/client/decodeWorker/types.tspackages/web-platform/web-core/ts/client/mainthread/TemplateManager.ts
✅ Files skipped from review due to trivial changes (1)
- .changeset/decode-worker-heartbreak.md
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/web-platform/web-core/ts/client/mainthread/TemplateManager.ts
- packages/web-platform/web-core/ts/client/decodeWorker/types.ts
- packages/web-platform/web-core/ts/client/decodeWorker/decode.worker.ts
Merging this PR will degrade performance by 12.93%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | 002-hello-reactLynx-destroyBackground |
863.4 µs | 912.4 µs | -5.37% |
| ❌ | transform 1000 view elements |
40 ms | 46 ms | -12.93% |
Comparing PupilTong:codex/decode-worker-heartbreak (b349c68) with main (d588d03)2
Footnotes
-
26 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
No successful run was found on
main(ad1f90f) during the generation of this report, so d588d03 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
React Example#8034 Bundle Size — 235.77KiB (0%).b349c68(current) vs 33b124f main#8029(baseline) Bundle metrics
|
| Current #8034 |
Baseline #8029 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
197 |
197 |
| Duplicate Modules |
80 |
80 |
| Duplicate Code |
44.85% |
44.85% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #8034 |
Baseline #8029 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
90.01KiB |
90.01KiB |
Bundle analysis report Branch PupilTong:codex/decode-worker-he... Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#1165 Bundle Size — 206.6KiB (0%).b349c68(current) vs 33b124f main#1160(baseline) Bundle metrics
|
| Current #1165 |
Baseline #1160 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
192 |
192 |
| Duplicate Modules |
77 |
77 |
| Duplicate Code |
44.36% |
44.36% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #1165 |
Baseline #1160 |
|
|---|---|---|
| IMG |
111.23KiB |
111.23KiB |
| Other |
95.37KiB |
95.37KiB |
Bundle analysis report Branch PupilTong:codex/decode-worker-he... Project dashboard
Generated by RelativeCI Documentation Report issue
React External#1148 Bundle Size — 690.27KiB (0%).b349c68(current) vs 33b124f main#1143(baseline) Bundle metrics
|
| Current #1148 |
Baseline #1143 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
17 |
17 |
| Duplicate Modules |
5 |
5 |
| Duplicate Code |
8.59% |
8.59% |
| Packages |
0 |
0 |
| Duplicate Packages |
0 |
0 |
Bundle analysis report Branch PupilTong:codex/decode-worker-he... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9607 Bundle Size — 900.31KiB (+0.03%).b349c68(current) vs 33b124f main#9602(baseline) Bundle metrics
|
| Current #9607 |
Baseline #9602 |
|
|---|---|---|
 Initial JS |
44.54KiB(+0.17%) |
44.46KiB |
| Initial CSS |
2.22KiB |
2.22KiB |
 Cache Invalidation |
8.57% |
0% |
| Chunks |
9 |
9 |
| Assets |
11 |
11 |
| Modules |
229 |
229 |
| Duplicate Modules |
11 |
11 |
| Duplicate Code |
27.27%(-0.04%) |
27.28% |
| Packages |
10 |
10 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 1 regression
| Current #9607 |
Baseline #9602 |
|
|---|---|---|
| JS |
496.17KiB (+0.05%) |
495.91KiB |
| Other |
401.92KiB |
401.92KiB |
| CSS |
2.22KiB |
2.22KiB |
Bundle analysis report Branch PupilTong:codex/decode-worker-he... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example with Element Template#300 Bundle Size — 197.79KiB (0%).b349c68(current) vs 33b124f main#295(baseline) Bundle metrics
|
| Current #300 |
Baseline #295 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
80(-1.23%) |
81 |
| Duplicate Modules |
23 |
23 |
| Duplicate Code |
40.31%(+0.05%) |
40.29% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #300 |
Baseline #295 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
52.03KiB |
52.03KiB |
Bundle analysis report Branch PupilTong:codex/decode-worker-he... Project dashboard
Generated by RelativeCI Documentation Report issue
PupilTong
force-pushed
the
codex/decode-worker-heartbreak
branch
from
120e984 to
b349c68
Compare
2 participants
Summary
Add a bidirectional worker-level
heartbreakmessage to the web-core decode worker.The decode worker starts one timer after it becomes ready. When the timer fires, the worker posts
{ type: 'heartbreak' }to the main thread and does not schedule another timer immediately.TemplateManagerreplies with{ type: 'heartbreak' }as soon as it receives the message. The worker starts the next timer only after it receives that reply, so there is at most one in-flight heartbreak at a time.The liveness message is independent of template URLs and does not interfere with existing URL-scoped
section,done, orerrorhandling.Validation
CI=1 pnpm install --frozen-lockfilepnpm dprint fmt packages/web-platform/web-core/ts/client/decodeWorker/types.ts packages/web-platform/web-core/ts/client/decodeWorker/decode.worker.ts packages/web-platform/web-core/ts/client/mainthread/TemplateManager.ts packages/web-platform/web-core/tests/template-manager.spec.ts .changeset/decode-worker-heartbreak.mdpnpm --dir packages/web-platform/web-core exec vitest run tests/template-manager.spec.ts(9 tests)git diff --checkNotes
pnpm turbo buildwas attempted before the focused test. It completed the web-core build, then failed later in unrelated@lynx-js/a2ui-reactlynxbecausepackages/genui/a2ui-catalog-extractor/dist/cli.jswas missing after a turbo cache hit.Summary by CodeRabbit
New Features
Bug Fixes
Tests