test(desktop): sample remount restore on painted frames - #3101

Merged
Astro-Han merged 3 commits into
apache:mainfrom
1625567290:fix/desktop-streaming-remount-observe
Aug 16, 2026
Merged

test(desktop): sample remount restore on painted frames#3101
Astro-Han merged 3 commits into
apache:mainfrom
1625567290:fix/desktop-streaming-remount-observe

Conversation

@1625567290

Copy link
Copy Markdown
Contributor

Summary

The "returning to a live conversation settles output accumulated while away" e2e sampled .maka-bubble-streaming on every document.body mutation. That observer sees React commit intermediates that never paint, so the "no partial catch-up" check depended on how much unrelated shell churn happened during remount. It passed in isolation and failed when the rest of the file had already warmed the machine.

The restore is now sampled on animation frames of the streaming bubble only. A real multi-frame replay would still fail; an intra-frame commit that the user never sees would not. The assertion also requires at least one painted sample to contain the settled background text, so a silent no-op observer cannot pass.

Fixes#3061

Verification

  • npx biome check apps/desktop/e2e/streaming-remount.spec.ts — clean
  • Isolated: playwright test e2e/streaming-remount.spec.ts --grep "accumulated while away"pass
  • Whole file after the change: 3/3, 3/3, plus one earlier whole-file run where this case still passed and an unrelated sibling (keeps a completed reply after an interrupted turn) timed out waiting for 停止 (pre-existing load flake, not this observer)
  • Before the change, this machine did not reproduce the original body-observer failure in 5 whole-file runs; the fix follows the diagnosis in the issue (observation too coarse), not a local red reproduction of that exact assertion

Not run: full desktop e2e suite, npm --workspace @maka/desktop test (no production code change)

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope:

Grok authored the test change and this PR description.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

The background-restore e2e watched document.body with a MutationObserver,
so every shell mutation during remount recorded .maka-bubble-streaming
textContent — including React commit intermediates that never painted.
That made the "no partial catch-up" assertion depend on compositor load:
it passed in isolation and failed when the rest of the file had already
warmed the machine.
Sample the bubble on animation frames instead, and require that at least
one painted sample contains the settled background text.
Fixesapache#3061
Generated-by: Grok
@coderabbitai

coderabbitaiBot commented Aug 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7cd6910e-da76-4235-9bf8-2171be824736

📥 Commits

Reviewing files that changed from the base of the PR and between 1339419 and 81add45.

📒 Files selected for processing (1)
  • apps/desktop/e2e/streaming-remount.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/desktop/e2e/streaming-remount.spec.ts

Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.


📝 Walkthrough

Summary

  • Fixes intermittent failures in the desktop background-restore E2E test.
  • Samples .maka-bubble-streaming on animation frames instead of all document.body mutations.
  • Avoids recording React commit intermediates that never paint.
  • Confirms that a painted sample contains the settled background text.
  • Confirms that no painted sample contains partially accumulated background output.

Design assessment

The change extends the existing E2E test. It does not create a parallel production path or change public contracts.

The frame sampler, sample deduplication, animation tracking, stoppable lifecycle, and next-frame stop are necessary to observe settled painted output reliably. No safe deletion or simplification is evident without weakening regression coverage.

Validation

  • Biome checks were run.
  • Repeated isolated and whole-file Playwright runs were performed.
  • The full desktop E2E suite and desktop unit tests were not run because production code did not change.
  • Required-check status remains unverified because direct current diff and status evidence is unavailable.

Review-relevant risks

No protected-area effect was identified in the current diff.

Walkthrough

The background-restore end-to-end test replaces document-wide mutation observation with a stoppable requestAnimationFrame sampler. It records distinct streaming text, tracks active animations, and stops sampling after the next animation frame.

Changes

Streaming restore test

Layer / File(s)Summary
Frame-based output sampling
apps/desktop/e2e/streaming-remount.spec.ts
The test samples the streaming bubble on animation frames, deduplicates text, tracks unfinished animations, waits for one final frame, stops the sampler, and validates restored output.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk:⚪ Minimal · up to 81add

This localized test-only change adjusts how remount output is sampled without changing product behavior, and no actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly describes the main change: sampling desktop remount restoration on painted animation frames.
Description check✅ PassedThe description follows the template, explains the problem and solution, lists verification, documents AI use, and records checklist decisions.
Linked Issues check✅ PassedThe test changes address issue #3061 by sampling painted frames and requiring settled output without partial catch-up states.
Out of Scope Changes check✅ PassedThe changes are limited to the related desktop streaming-remount E2E test and its observation logic.
Ai Use Disclosure✅ PassedThe PR selects generative tooling and names Grok with scope. Both introduced commits contain standalone Generated-by: Grok trailers.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: cc9670e7-a884-4981-8b6d-710594b20c29

📥 Commits

Reviewing files that changed from the base of the PR and between 9fbdc99 and 1339419.

📒 Files selected for processing (1)
  • apps/desktop/e2e/streaming-remount.spec.ts

Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.

Comment threadapps/desktop/e2e/streaming-remount.spec.ts
toContainText observes the DOM, not the rAF sampler. Stopping
synchronously could drop the settled paint if it landed after the last
queued sample. Stop from the next animation frame so that sample records
first.
Generated-by: Grok
@Astro-Han

Copy link
Copy Markdown
Contributor

Fast-path merge

This change qualifies for the self-merge fast path: it is low impact and easy to reverse (test-only change sampling the remount restore on painted animation frames instead of every DOM mutation), does not touch the protected areas (Runtime Host execution authority, @maka/eval semantics, public Eval CLI, security, licensing, releases, governance), and passes all required checks.

The latest head was independently reviewed by a read-only subagent (ollama-cloud/deepseek-v4-flash:high) with no open P0/P1/P2 findings. Human contributor @yuhan reviewed the final diff and chose the fast path.

@Astro-Han
Astro-Han merged commit 84a579d into apache:mainAug 16, 2026
12 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

streaming-remount background-restore test is flaky on macOS

2 participants

@1625567290@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

test(desktop): sample remount restore on painted frames - #3101

Merged
Astro-Han merged 3 commits into
apache:mainfrom
1625567290:fix/desktop-streaming-remount-observe
Aug 16, 2026
Merged

test(desktop): sample remount restore on painted frames#3101
Astro-Han merged 3 commits into
apache:mainfrom
1625567290:fix/desktop-streaming-remount-observe

Conversation

@1625567290

Copy link
Copy Markdown
Contributor

Summary

The "returning to a live conversation settles output accumulated while away" e2e sampled .maka-bubble-streaming on every document.body mutation. That observer sees React commit intermediates that never paint, so the "no partial catch-up" check depended on how much unrelated shell churn happened during remount. It passed in isolation and failed when the rest of the file had already warmed the machine.

The restore is now sampled on animation frames of the streaming bubble only. A real multi-frame replay would still fail; an intra-frame commit that the user never sees would not. The assertion also requires at least one painted sample to contain the settled background text, so a silent no-op observer cannot pass.

Fixes#3061

Verification

  • npx biome check apps/desktop/e2e/streaming-remount.spec.ts — clean
  • Isolated: playwright test e2e/streaming-remount.spec.ts --grep "accumulated while away"pass
  • Whole file after the change: 3/3, 3/3, plus one earlier whole-file run where this case still passed and an unrelated sibling (keeps a completed reply after an interrupted turn) timed out waiting for 停止 (pre-existing load flake, not this observer)
  • Before the change, this machine did not reproduce the original body-observer failure in 5 whole-file runs; the fix follows the diagnosis in the issue (observation too coarse), not a local red reproduction of that exact assertion

Not run: full desktop e2e suite, npm --workspace @maka/desktop test (no production code change)

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope:

Grok authored the test change and this PR description.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

The background-restore e2e watched document.body with a MutationObserver,
so every shell mutation during remount recorded .maka-bubble-streaming
textContent — including React commit intermediates that never painted.
That made the "no partial catch-up" assertion depend on compositor load:
it passed in isolation and failed when the rest of the file had already
warmed the machine.
Sample the bubble on animation frames instead, and require that at least
one painted sample contains the settled background text.
Fixesapache#3061
Generated-by: Grok
@coderabbitai

coderabbitaiBot commented Aug 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7cd6910e-da76-4235-9bf8-2171be824736

📥 Commits

Reviewing files that changed from the base of the PR and between 1339419 and 81add45.

📒 Files selected for processing (1)
  • apps/desktop/e2e/streaming-remount.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/desktop/e2e/streaming-remount.spec.ts

Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.


📝 Walkthrough

Summary

  • Fixes intermittent failures in the desktop background-restore E2E test.
  • Samples .maka-bubble-streaming on animation frames instead of all document.body mutations.
  • Avoids recording React commit intermediates that never paint.
  • Confirms that a painted sample contains the settled background text.
  • Confirms that no painted sample contains partially accumulated background output.

Design assessment

The change extends the existing E2E test. It does not create a parallel production path or change public contracts.

The frame sampler, sample deduplication, animation tracking, stoppable lifecycle, and next-frame stop are necessary to observe settled painted output reliably. No safe deletion or simplification is evident without weakening regression coverage.

Validation

  • Biome checks were run.
  • Repeated isolated and whole-file Playwright runs were performed.
  • The full desktop E2E suite and desktop unit tests were not run because production code did not change.
  • Required-check status remains unverified because direct current diff and status evidence is unavailable.

Review-relevant risks

No protected-area effect was identified in the current diff.

Walkthrough

The background-restore end-to-end test replaces document-wide mutation observation with a stoppable requestAnimationFrame sampler. It records distinct streaming text, tracks active animations, and stops sampling after the next animation frame.

Changes

Streaming restore test

Layer / File(s)Summary
Frame-based output sampling
apps/desktop/e2e/streaming-remount.spec.ts
The test samples the streaming bubble on animation frames, deduplicates text, tracks unfinished animations, waits for one final frame, stops the sampler, and validates restored output.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk:⚪ Minimal · up to 81add

This localized test-only change adjusts how remount output is sampled without changing product behavior, and no actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly describes the main change: sampling desktop remount restoration on painted animation frames.
Description check✅ PassedThe description follows the template, explains the problem and solution, lists verification, documents AI use, and records checklist decisions.
Linked Issues check✅ PassedThe test changes address issue #3061 by sampling painted frames and requiring settled output without partial catch-up states.
Out of Scope Changes check✅ PassedThe changes are limited to the related desktop streaming-remount E2E test and its observation logic.
Ai Use Disclosure✅ PassedThe PR selects generative tooling and names Grok with scope. Both introduced commits contain standalone Generated-by: Grok trailers.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: cc9670e7-a884-4981-8b6d-710594b20c29

📥 Commits

Reviewing files that changed from the base of the PR and between 9fbdc99 and 1339419.

📒 Files selected for processing (1)
  • apps/desktop/e2e/streaming-remount.spec.ts

Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.

Comment threadapps/desktop/e2e/streaming-remount.spec.ts
toContainText observes the DOM, not the rAF sampler. Stopping
synchronously could drop the settled paint if it landed after the last
queued sample. Stop from the next animation frame so that sample records
first.
Generated-by: Grok
@Astro-Han

Copy link
Copy Markdown
Contributor

Fast-path merge

This change qualifies for the self-merge fast path: it is low impact and easy to reverse (test-only change sampling the remount restore on painted animation frames instead of every DOM mutation), does not touch the protected areas (Runtime Host execution authority, @maka/eval semantics, public Eval CLI, security, licensing, releases, governance), and passes all required checks.

The latest head was independently reviewed by a read-only subagent (ollama-cloud/deepseek-v4-flash:high) with no open P0/P1/P2 findings. Human contributor @yuhan reviewed the final diff and chose the fast path.

@Astro-Han
Astro-Han merged commit 84a579d into apache:mainAug 16, 2026
12 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

streaming-remount background-restore test is flaky on macOS

2 participants

@1625567290@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

test(desktop): sample remount restore on painted frames - #3101

Merged
Astro-Han merged 3 commits into
apache:mainfrom
1625567290:fix/desktop-streaming-remount-observe
Aug 16, 2026
Merged

test(desktop): sample remount restore on painted frames#3101
Astro-Han merged 3 commits into
apache:mainfrom
1625567290:fix/desktop-streaming-remount-observe

Conversation

@1625567290

Copy link
Copy Markdown
Contributor

Summary

The "returning to a live conversation settles output accumulated while away" e2e sampled .maka-bubble-streaming on every document.body mutation. That observer sees React commit intermediates that never paint, so the "no partial catch-up" check depended on how much unrelated shell churn happened during remount. It passed in isolation and failed when the rest of the file had already warmed the machine.

The restore is now sampled on animation frames of the streaming bubble only. A real multi-frame replay would still fail; an intra-frame commit that the user never sees would not. The assertion also requires at least one painted sample to contain the settled background text, so a silent no-op observer cannot pass.

Fixes#3061

Verification

  • npx biome check apps/desktop/e2e/streaming-remount.spec.ts — clean
  • Isolated: playwright test e2e/streaming-remount.spec.ts --grep "accumulated while away"pass
  • Whole file after the change: 3/3, 3/3, plus one earlier whole-file run where this case still passed and an unrelated sibling (keeps a completed reply after an interrupted turn) timed out waiting for 停止 (pre-existing load flake, not this observer)
  • Before the change, this machine did not reproduce the original body-observer failure in 5 whole-file runs; the fix follows the diagnosis in the issue (observation too coarse), not a local red reproduction of that exact assertion

Not run: full desktop e2e suite, npm --workspace @maka/desktop test (no production code change)

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope:

Grok authored the test change and this PR description.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

The background-restore e2e watched document.body with a MutationObserver,
so every shell mutation during remount recorded .maka-bubble-streaming
textContent — including React commit intermediates that never painted.
That made the "no partial catch-up" assertion depend on compositor load:
it passed in isolation and failed when the rest of the file had already
warmed the machine.
Sample the bubble on animation frames instead, and require that at least
one painted sample contains the settled background text.
Fixesapache#3061
Generated-by: Grok
@coderabbitai

coderabbitaiBot commented Aug 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7cd6910e-da76-4235-9bf8-2171be824736

📥 Commits

Reviewing files that changed from the base of the PR and between 1339419 and 81add45.

📒 Files selected for processing (1)
  • apps/desktop/e2e/streaming-remount.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/desktop/e2e/streaming-remount.spec.ts

Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.


📝 Walkthrough

Summary

  • Fixes intermittent failures in the desktop background-restore E2E test.
  • Samples .maka-bubble-streaming on animation frames instead of all document.body mutations.
  • Avoids recording React commit intermediates that never paint.
  • Confirms that a painted sample contains the settled background text.
  • Confirms that no painted sample contains partially accumulated background output.

Design assessment

The change extends the existing E2E test. It does not create a parallel production path or change public contracts.

The frame sampler, sample deduplication, animation tracking, stoppable lifecycle, and next-frame stop are necessary to observe settled painted output reliably. No safe deletion or simplification is evident without weakening regression coverage.

Validation

  • Biome checks were run.
  • Repeated isolated and whole-file Playwright runs were performed.
  • The full desktop E2E suite and desktop unit tests were not run because production code did not change.
  • Required-check status remains unverified because direct current diff and status evidence is unavailable.

Review-relevant risks

No protected-area effect was identified in the current diff.

Walkthrough

The background-restore end-to-end test replaces document-wide mutation observation with a stoppable requestAnimationFrame sampler. It records distinct streaming text, tracks active animations, and stops sampling after the next animation frame.

Changes

Streaming restore test

Layer / File(s)Summary
Frame-based output sampling
apps/desktop/e2e/streaming-remount.spec.ts
The test samples the streaming bubble on animation frames, deduplicates text, tracks unfinished animations, waits for one final frame, stops the sampler, and validates restored output.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk:⚪ Minimal · up to 81add

This localized test-only change adjusts how remount output is sampled without changing product behavior, and no actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly describes the main change: sampling desktop remount restoration on painted animation frames.
Description check✅ PassedThe description follows the template, explains the problem and solution, lists verification, documents AI use, and records checklist decisions.
Linked Issues check✅ PassedThe test changes address issue #3061 by sampling painted frames and requiring settled output without partial catch-up states.
Out of Scope Changes check✅ PassedThe changes are limited to the related desktop streaming-remount E2E test and its observation logic.
Ai Use Disclosure✅ PassedThe PR selects generative tooling and names Grok with scope. Both introduced commits contain standalone Generated-by: Grok trailers.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: cc9670e7-a884-4981-8b6d-710594b20c29

📥 Commits

Reviewing files that changed from the base of the PR and between 9fbdc99 and 1339419.

📒 Files selected for processing (1)
  • apps/desktop/e2e/streaming-remount.spec.ts

Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.

Comment threadapps/desktop/e2e/streaming-remount.spec.ts
toContainText observes the DOM, not the rAF sampler. Stopping
synchronously could drop the settled paint if it landed after the last
queued sample. Stop from the next animation frame so that sample records
first.
Generated-by: Grok
@Astro-Han

Copy link
Copy Markdown
Contributor

Fast-path merge

This change qualifies for the self-merge fast path: it is low impact and easy to reverse (test-only change sampling the remount restore on painted animation frames instead of every DOM mutation), does not touch the protected areas (Runtime Host execution authority, @maka/eval semantics, public Eval CLI, security, licensing, releases, governance), and passes all required checks.

The latest head was independently reviewed by a read-only subagent (ollama-cloud/deepseek-v4-flash:high) with no open P0/P1/P2 findings. Human contributor @yuhan reviewed the final diff and chose the fast path.

@Astro-Han
Astro-Han merged commit 84a579d into apache:mainAug 16, 2026
12 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

streaming-remount background-restore test is flaky on macOS

2 participants

@1625567290@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

test(desktop): sample remount restore on painted frames - #3101

Merged
Astro-Han merged 3 commits into
apache:mainfrom
1625567290:fix/desktop-streaming-remount-observe
Aug 16, 2026
Merged

test(desktop): sample remount restore on painted frames#3101
Astro-Han merged 3 commits into
apache:mainfrom
1625567290:fix/desktop-streaming-remount-observe

Conversation

@1625567290

Copy link
Copy Markdown
Contributor

Summary

The "returning to a live conversation settles output accumulated while away" e2e sampled .maka-bubble-streaming on every document.body mutation. That observer sees React commit intermediates that never paint, so the "no partial catch-up" check depended on how much unrelated shell churn happened during remount. It passed in isolation and failed when the rest of the file had already warmed the machine.

The restore is now sampled on animation frames of the streaming bubble only. A real multi-frame replay would still fail; an intra-frame commit that the user never sees would not. The assertion also requires at least one painted sample to contain the settled background text, so a silent no-op observer cannot pass.

Fixes#3061

Verification

  • npx biome check apps/desktop/e2e/streaming-remount.spec.ts — clean
  • Isolated: playwright test e2e/streaming-remount.spec.ts --grep "accumulated while away"pass
  • Whole file after the change: 3/3, 3/3, plus one earlier whole-file run where this case still passed and an unrelated sibling (keeps a completed reply after an interrupted turn) timed out waiting for 停止 (pre-existing load flake, not this observer)
  • Before the change, this machine did not reproduce the original body-observer failure in 5 whole-file runs; the fix follows the diagnosis in the issue (observation too coarse), not a local red reproduction of that exact assertion

Not run: full desktop e2e suite, npm --workspace @maka/desktop test (no production code change)

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope:

Grok authored the test change and this PR description.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

The background-restore e2e watched document.body with a MutationObserver,
so every shell mutation during remount recorded .maka-bubble-streaming
textContent — including React commit intermediates that never painted.
That made the "no partial catch-up" assertion depend on compositor load:
it passed in isolation and failed when the rest of the file had already
warmed the machine.
Sample the bubble on animation frames instead, and require that at least
one painted sample contains the settled background text.
Fixesapache#3061
Generated-by: Grok
@coderabbitai

coderabbitaiBot commented Aug 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7cd6910e-da76-4235-9bf8-2171be824736

📥 Commits

Reviewing files that changed from the base of the PR and between 1339419 and 81add45.

📒 Files selected for processing (1)
  • apps/desktop/e2e/streaming-remount.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/desktop/e2e/streaming-remount.spec.ts

Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.


📝 Walkthrough

Summary

  • Fixes intermittent failures in the desktop background-restore E2E test.
  • Samples .maka-bubble-streaming on animation frames instead of all document.body mutations.
  • Avoids recording React commit intermediates that never paint.
  • Confirms that a painted sample contains the settled background text.
  • Confirms that no painted sample contains partially accumulated background output.

Design assessment

The change extends the existing E2E test. It does not create a parallel production path or change public contracts.

The frame sampler, sample deduplication, animation tracking, stoppable lifecycle, and next-frame stop are necessary to observe settled painted output reliably. No safe deletion or simplification is evident without weakening regression coverage.

Validation

  • Biome checks were run.
  • Repeated isolated and whole-file Playwright runs were performed.
  • The full desktop E2E suite and desktop unit tests were not run because production code did not change.
  • Required-check status remains unverified because direct current diff and status evidence is unavailable.

Review-relevant risks

No protected-area effect was identified in the current diff.

Walkthrough

The background-restore end-to-end test replaces document-wide mutation observation with a stoppable requestAnimationFrame sampler. It records distinct streaming text, tracks active animations, and stops sampling after the next animation frame.

Changes

Streaming restore test

Layer / File(s)Summary
Frame-based output sampling
apps/desktop/e2e/streaming-remount.spec.ts
The test samples the streaming bubble on animation frames, deduplicates text, tracks unfinished animations, waits for one final frame, stops the sampler, and validates restored output.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk:⚪ Minimal · up to 81add

This localized test-only change adjusts how remount output is sampled without changing product behavior, and no actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly describes the main change: sampling desktop remount restoration on painted animation frames.
Description check✅ PassedThe description follows the template, explains the problem and solution, lists verification, documents AI use, and records checklist decisions.
Linked Issues check✅ PassedThe test changes address issue #3061 by sampling painted frames and requiring settled output without partial catch-up states.
Out of Scope Changes check✅ PassedThe changes are limited to the related desktop streaming-remount E2E test and its observation logic.
Ai Use Disclosure✅ PassedThe PR selects generative tooling and names Grok with scope. Both introduced commits contain standalone Generated-by: Grok trailers.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: cc9670e7-a884-4981-8b6d-710594b20c29

📥 Commits

Reviewing files that changed from the base of the PR and between 9fbdc99 and 1339419.

📒 Files selected for processing (1)
  • apps/desktop/e2e/streaming-remount.spec.ts

Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.

Comment threadapps/desktop/e2e/streaming-remount.spec.ts
toContainText observes the DOM, not the rAF sampler. Stopping
synchronously could drop the settled paint if it landed after the last
queued sample. Stop from the next animation frame so that sample records
first.
Generated-by: Grok
@Astro-Han

Copy link
Copy Markdown
Contributor

Fast-path merge

This change qualifies for the self-merge fast path: it is low impact and easy to reverse (test-only change sampling the remount restore on painted animation frames instead of every DOM mutation), does not touch the protected areas (Runtime Host execution authority, @maka/eval semantics, public Eval CLI, security, licensing, releases, governance), and passes all required checks.

The latest head was independently reviewed by a read-only subagent (ollama-cloud/deepseek-v4-flash:high) with no open P0/P1/P2 findings. Human contributor @yuhan reviewed the final diff and chose the fast path.

@Astro-Han
Astro-Han merged commit 84a579d into apache:mainAug 16, 2026
12 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

streaming-remount background-restore test is flaky on macOS

2 participants

@1625567290@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

test(desktop): sample remount restore on painted frames - #3101

Merged
Astro-Han merged 3 commits into
apache:mainfrom
1625567290:fix/desktop-streaming-remount-observe
Aug 16, 2026
Merged

test(desktop): sample remount restore on painted frames#3101
Astro-Han merged 3 commits into
apache:mainfrom
1625567290:fix/desktop-streaming-remount-observe

Conversation

@1625567290

Copy link
Copy Markdown
Contributor

Summary

The "returning to a live conversation settles output accumulated while away" e2e sampled .maka-bubble-streaming on every document.body mutation. That observer sees React commit intermediates that never paint, so the "no partial catch-up" check depended on how much unrelated shell churn happened during remount. It passed in isolation and failed when the rest of the file had already warmed the machine.

The restore is now sampled on animation frames of the streaming bubble only. A real multi-frame replay would still fail; an intra-frame commit that the user never sees would not. The assertion also requires at least one painted sample to contain the settled background text, so a silent no-op observer cannot pass.

Fixes#3061

Verification

  • npx biome check apps/desktop/e2e/streaming-remount.spec.ts — clean
  • Isolated: playwright test e2e/streaming-remount.spec.ts --grep "accumulated while away"pass
  • Whole file after the change: 3/3, 3/3, plus one earlier whole-file run where this case still passed and an unrelated sibling (keeps a completed reply after an interrupted turn) timed out waiting for 停止 (pre-existing load flake, not this observer)
  • Before the change, this machine did not reproduce the original body-observer failure in 5 whole-file runs; the fix follows the diagnosis in the issue (observation too coarse), not a local red reproduction of that exact assertion

Not run: full desktop e2e suite, npm --workspace @maka/desktop test (no production code change)

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope:

Grok authored the test change and this PR description.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

The background-restore e2e watched document.body with a MutationObserver,
so every shell mutation during remount recorded .maka-bubble-streaming
textContent — including React commit intermediates that never painted.
That made the "no partial catch-up" assertion depend on compositor load:
it passed in isolation and failed when the rest of the file had already
warmed the machine.
Sample the bubble on animation frames instead, and require that at least
one painted sample contains the settled background text.
Fixesapache#3061
Generated-by: Grok
@coderabbitai

coderabbitaiBot commented Aug 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7cd6910e-da76-4235-9bf8-2171be824736

📥 Commits

Reviewing files that changed from the base of the PR and between 1339419 and 81add45.

📒 Files selected for processing (1)
  • apps/desktop/e2e/streaming-remount.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/desktop/e2e/streaming-remount.spec.ts

Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.


📝 Walkthrough

Summary

  • Fixes intermittent failures in the desktop background-restore E2E test.
  • Samples .maka-bubble-streaming on animation frames instead of all document.body mutations.
  • Avoids recording React commit intermediates that never paint.
  • Confirms that a painted sample contains the settled background text.
  • Confirms that no painted sample contains partially accumulated background output.

Design assessment

The change extends the existing E2E test. It does not create a parallel production path or change public contracts.

The frame sampler, sample deduplication, animation tracking, stoppable lifecycle, and next-frame stop are necessary to observe settled painted output reliably. No safe deletion or simplification is evident without weakening regression coverage.

Validation

  • Biome checks were run.
  • Repeated isolated and whole-file Playwright runs were performed.
  • The full desktop E2E suite and desktop unit tests were not run because production code did not change.
  • Required-check status remains unverified because direct current diff and status evidence is unavailable.

Review-relevant risks

No protected-area effect was identified in the current diff.

Walkthrough

The background-restore end-to-end test replaces document-wide mutation observation with a stoppable requestAnimationFrame sampler. It records distinct streaming text, tracks active animations, and stops sampling after the next animation frame.

Changes

Streaming restore test

Layer / File(s)Summary
Frame-based output sampling
apps/desktop/e2e/streaming-remount.spec.ts
The test samples the streaming bubble on animation frames, deduplicates text, tracks unfinished animations, waits for one final frame, stops the sampler, and validates restored output.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk:⚪ Minimal · up to 81add

This localized test-only change adjusts how remount output is sampled without changing product behavior, and no actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly describes the main change: sampling desktop remount restoration on painted animation frames.
Description check✅ PassedThe description follows the template, explains the problem and solution, lists verification, documents AI use, and records checklist decisions.
Linked Issues check✅ PassedThe test changes address issue #3061 by sampling painted frames and requiring settled output without partial catch-up states.
Out of Scope Changes check✅ PassedThe changes are limited to the related desktop streaming-remount E2E test and its observation logic.
Ai Use Disclosure✅ PassedThe PR selects generative tooling and names Grok with scope. Both introduced commits contain standalone Generated-by: Grok trailers.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: cc9670e7-a884-4981-8b6d-710594b20c29

📥 Commits

Reviewing files that changed from the base of the PR and between 9fbdc99 and 1339419.

📒 Files selected for processing (1)
  • apps/desktop/e2e/streaming-remount.spec.ts

Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.

Comment threadapps/desktop/e2e/streaming-remount.spec.ts
toContainText observes the DOM, not the rAF sampler. Stopping
synchronously could drop the settled paint if it landed after the last
queued sample. Stop from the next animation frame so that sample records
first.
Generated-by: Grok
@Astro-Han

Copy link
Copy Markdown
Contributor

Fast-path merge

This change qualifies for the self-merge fast path: it is low impact and easy to reverse (test-only change sampling the remount restore on painted animation frames instead of every DOM mutation), does not touch the protected areas (Runtime Host execution authority, @maka/eval semantics, public Eval CLI, security, licensing, releases, governance), and passes all required checks.

The latest head was independently reviewed by a read-only subagent (ollama-cloud/deepseek-v4-flash:high) with no open P0/P1/P2 findings. Human contributor @yuhan reviewed the final diff and chose the fast path.

@Astro-Han
Astro-Han merged commit 84a579d into apache:mainAug 16, 2026
12 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

streaming-remount background-restore test is flaky on macOS

2 participants

@1625567290@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

test(desktop): sample remount restore on painted frames - #3101

Merged
Astro-Han merged 3 commits into
apache:mainfrom
1625567290:fix/desktop-streaming-remount-observe
Aug 16, 2026
Merged

test(desktop): sample remount restore on painted frames#3101
Astro-Han merged 3 commits into
apache:mainfrom
1625567290:fix/desktop-streaming-remount-observe

Conversation

@1625567290

Copy link
Copy Markdown
Contributor

Summary

The "returning to a live conversation settles output accumulated while away" e2e sampled .maka-bubble-streaming on every document.body mutation. That observer sees React commit intermediates that never paint, so the "no partial catch-up" check depended on how much unrelated shell churn happened during remount. It passed in isolation and failed when the rest of the file had already warmed the machine.

The restore is now sampled on animation frames of the streaming bubble only. A real multi-frame replay would still fail; an intra-frame commit that the user never sees would not. The assertion also requires at least one painted sample to contain the settled background text, so a silent no-op observer cannot pass.

Fixes#3061

Verification

  • npx biome check apps/desktop/e2e/streaming-remount.spec.ts — clean
  • Isolated: playwright test e2e/streaming-remount.spec.ts --grep "accumulated while away"pass
  • Whole file after the change: 3/3, 3/3, plus one earlier whole-file run where this case still passed and an unrelated sibling (keeps a completed reply after an interrupted turn) timed out waiting for 停止 (pre-existing load flake, not this observer)
  • Before the change, this machine did not reproduce the original body-observer failure in 5 whole-file runs; the fix follows the diagnosis in the issue (observation too coarse), not a local red reproduction of that exact assertion

Not run: full desktop e2e suite, npm --workspace @maka/desktop test (no production code change)

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope:

Grok authored the test change and this PR description.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

The background-restore e2e watched document.body with a MutationObserver,
so every shell mutation during remount recorded .maka-bubble-streaming
textContent — including React commit intermediates that never painted.
That made the "no partial catch-up" assertion depend on compositor load:
it passed in isolation and failed when the rest of the file had already
warmed the machine.
Sample the bubble on animation frames instead, and require that at least
one painted sample contains the settled background text.
Fixesapache#3061
Generated-by: Grok
@coderabbitai

coderabbitaiBot commented Aug 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7cd6910e-da76-4235-9bf8-2171be824736

📥 Commits

Reviewing files that changed from the base of the PR and between 1339419 and 81add45.

📒 Files selected for processing (1)
  • apps/desktop/e2e/streaming-remount.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/desktop/e2e/streaming-remount.spec.ts

Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.


📝 Walkthrough

Summary

  • Fixes intermittent failures in the desktop background-restore E2E test.
  • Samples .maka-bubble-streaming on animation frames instead of all document.body mutations.
  • Avoids recording React commit intermediates that never paint.
  • Confirms that a painted sample contains the settled background text.
  • Confirms that no painted sample contains partially accumulated background output.

Design assessment

The change extends the existing E2E test. It does not create a parallel production path or change public contracts.

The frame sampler, sample deduplication, animation tracking, stoppable lifecycle, and next-frame stop are necessary to observe settled painted output reliably. No safe deletion or simplification is evident without weakening regression coverage.

Validation

  • Biome checks were run.
  • Repeated isolated and whole-file Playwright runs were performed.
  • The full desktop E2E suite and desktop unit tests were not run because production code did not change.
  • Required-check status remains unverified because direct current diff and status evidence is unavailable.

Review-relevant risks

No protected-area effect was identified in the current diff.

Walkthrough

The background-restore end-to-end test replaces document-wide mutation observation with a stoppable requestAnimationFrame sampler. It records distinct streaming text, tracks active animations, and stops sampling after the next animation frame.

Changes

Streaming restore test

Layer / File(s)Summary
Frame-based output sampling
apps/desktop/e2e/streaming-remount.spec.ts
The test samples the streaming bubble on animation frames, deduplicates text, tracks unfinished animations, waits for one final frame, stops the sampler, and validates restored output.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk:⚪ Minimal · up to 81add

This localized test-only change adjusts how remount output is sampled without changing product behavior, and no actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly describes the main change: sampling desktop remount restoration on painted animation frames.
Description check✅ PassedThe description follows the template, explains the problem and solution, lists verification, documents AI use, and records checklist decisions.
Linked Issues check✅ PassedThe test changes address issue #3061 by sampling painted frames and requiring settled output without partial catch-up states.
Out of Scope Changes check✅ PassedThe changes are limited to the related desktop streaming-remount E2E test and its observation logic.
Ai Use Disclosure✅ PassedThe PR selects generative tooling and names Grok with scope. Both introduced commits contain standalone Generated-by: Grok trailers.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: cc9670e7-a884-4981-8b6d-710594b20c29

📥 Commits

Reviewing files that changed from the base of the PR and between 9fbdc99 and 1339419.

📒 Files selected for processing (1)
  • apps/desktop/e2e/streaming-remount.spec.ts

Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.

Comment threadapps/desktop/e2e/streaming-remount.spec.ts
toContainText observes the DOM, not the rAF sampler. Stopping
synchronously could drop the settled paint if it landed after the last
queued sample. Stop from the next animation frame so that sample records
first.
Generated-by: Grok
@Astro-Han

Copy link
Copy Markdown
Contributor

Fast-path merge

This change qualifies for the self-merge fast path: it is low impact and easy to reverse (test-only change sampling the remount restore on painted animation frames instead of every DOM mutation), does not touch the protected areas (Runtime Host execution authority, @maka/eval semantics, public Eval CLI, security, licensing, releases, governance), and passes all required checks.

The latest head was independently reviewed by a read-only subagent (ollama-cloud/deepseek-v4-flash:high) with no open P0/P1/P2 findings. Human contributor @yuhan reviewed the final diff and chose the fast path.

@Astro-Han
Astro-Han merged commit 84a579d into apache:mainAug 16, 2026
12 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

streaming-remount background-restore test is flaky on macOS

2 participants

@1625567290@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

test(desktop): sample remount restore on painted frames - #3101

Merged
Astro-Han merged 3 commits into
apache:mainfrom
1625567290:fix/desktop-streaming-remount-observe
Aug 16, 2026
Merged

test(desktop): sample remount restore on painted frames#3101
Astro-Han merged 3 commits into
apache:mainfrom
1625567290:fix/desktop-streaming-remount-observe

Conversation

@1625567290

Copy link
Copy Markdown
Contributor

Summary

The "returning to a live conversation settles output accumulated while away" e2e sampled .maka-bubble-streaming on every document.body mutation. That observer sees React commit intermediates that never paint, so the "no partial catch-up" check depended on how much unrelated shell churn happened during remount. It passed in isolation and failed when the rest of the file had already warmed the machine.

The restore is now sampled on animation frames of the streaming bubble only. A real multi-frame replay would still fail; an intra-frame commit that the user never sees would not. The assertion also requires at least one painted sample to contain the settled background text, so a silent no-op observer cannot pass.

Fixes#3061

Verification

  • npx biome check apps/desktop/e2e/streaming-remount.spec.ts — clean
  • Isolated: playwright test e2e/streaming-remount.spec.ts --grep "accumulated while away"pass
  • Whole file after the change: 3/3, 3/3, plus one earlier whole-file run where this case still passed and an unrelated sibling (keeps a completed reply after an interrupted turn) timed out waiting for 停止 (pre-existing load flake, not this observer)
  • Before the change, this machine did not reproduce the original body-observer failure in 5 whole-file runs; the fix follows the diagnosis in the issue (observation too coarse), not a local red reproduction of that exact assertion

Not run: full desktop e2e suite, npm --workspace @maka/desktop test (no production code change)

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope:

Grok authored the test change and this PR description.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

The background-restore e2e watched document.body with a MutationObserver,
so every shell mutation during remount recorded .maka-bubble-streaming
textContent — including React commit intermediates that never painted.
That made the "no partial catch-up" assertion depend on compositor load:
it passed in isolation and failed when the rest of the file had already
warmed the machine.
Sample the bubble on animation frames instead, and require that at least
one painted sample contains the settled background text.
Fixesapache#3061
Generated-by: Grok
@coderabbitai

coderabbitaiBot commented Aug 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7cd6910e-da76-4235-9bf8-2171be824736

📥 Commits

Reviewing files that changed from the base of the PR and between 1339419 and 81add45.

📒 Files selected for processing (1)
  • apps/desktop/e2e/streaming-remount.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/desktop/e2e/streaming-remount.spec.ts

Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.


📝 Walkthrough

Summary

  • Fixes intermittent failures in the desktop background-restore E2E test.
  • Samples .maka-bubble-streaming on animation frames instead of all document.body mutations.
  • Avoids recording React commit intermediates that never paint.
  • Confirms that a painted sample contains the settled background text.
  • Confirms that no painted sample contains partially accumulated background output.

Design assessment

The change extends the existing E2E test. It does not create a parallel production path or change public contracts.

The frame sampler, sample deduplication, animation tracking, stoppable lifecycle, and next-frame stop are necessary to observe settled painted output reliably. No safe deletion or simplification is evident without weakening regression coverage.

Validation

  • Biome checks were run.
  • Repeated isolated and whole-file Playwright runs were performed.
  • The full desktop E2E suite and desktop unit tests were not run because production code did not change.
  • Required-check status remains unverified because direct current diff and status evidence is unavailable.

Review-relevant risks

No protected-area effect was identified in the current diff.

Walkthrough

The background-restore end-to-end test replaces document-wide mutation observation with a stoppable requestAnimationFrame sampler. It records distinct streaming text, tracks active animations, and stops sampling after the next animation frame.

Changes

Streaming restore test

Layer / File(s)Summary
Frame-based output sampling
apps/desktop/e2e/streaming-remount.spec.ts
The test samples the streaming bubble on animation frames, deduplicates text, tracks unfinished animations, waits for one final frame, stops the sampler, and validates restored output.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk:⚪ Minimal · up to 81add

This localized test-only change adjusts how remount output is sampled without changing product behavior, and no actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly describes the main change: sampling desktop remount restoration on painted animation frames.
Description check✅ PassedThe description follows the template, explains the problem and solution, lists verification, documents AI use, and records checklist decisions.
Linked Issues check✅ PassedThe test changes address issue #3061 by sampling painted frames and requiring settled output without partial catch-up states.
Out of Scope Changes check✅ PassedThe changes are limited to the related desktop streaming-remount E2E test and its observation logic.
Ai Use Disclosure✅ PassedThe PR selects generative tooling and names Grok with scope. Both introduced commits contain standalone Generated-by: Grok trailers.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: cc9670e7-a884-4981-8b6d-710594b20c29

📥 Commits

Reviewing files that changed from the base of the PR and between 9fbdc99 and 1339419.

📒 Files selected for processing (1)
  • apps/desktop/e2e/streaming-remount.spec.ts

Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.

Comment threadapps/desktop/e2e/streaming-remount.spec.ts
toContainText observes the DOM, not the rAF sampler. Stopping
synchronously could drop the settled paint if it landed after the last
queued sample. Stop from the next animation frame so that sample records
first.
Generated-by: Grok
@Astro-Han

Copy link
Copy Markdown
Contributor

Fast-path merge

This change qualifies for the self-merge fast path: it is low impact and easy to reverse (test-only change sampling the remount restore on painted animation frames instead of every DOM mutation), does not touch the protected areas (Runtime Host execution authority, @maka/eval semantics, public Eval CLI, security, licensing, releases, governance), and passes all required checks.

The latest head was independently reviewed by a read-only subagent (ollama-cloud/deepseek-v4-flash:high) with no open P0/P1/P2 findings. Human contributor @yuhan reviewed the final diff and chose the fast path.

@Astro-Han
Astro-Han merged commit 84a579d into apache:mainAug 16, 2026
12 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

streaming-remount background-restore test is flaky on macOS

2 participants

@1625567290@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

test(desktop): sample remount restore on painted frames - #3101

Merged
Astro-Han merged 3 commits into
apache:mainfrom
1625567290:fix/desktop-streaming-remount-observe
Aug 16, 2026
Merged

test(desktop): sample remount restore on painted frames#3101
Astro-Han merged 3 commits into
apache:mainfrom
1625567290:fix/desktop-streaming-remount-observe

Conversation

@1625567290

Copy link
Copy Markdown
Contributor

Summary

The "returning to a live conversation settles output accumulated while away" e2e sampled .maka-bubble-streaming on every document.body mutation. That observer sees React commit intermediates that never paint, so the "no partial catch-up" check depended on how much unrelated shell churn happened during remount. It passed in isolation and failed when the rest of the file had already warmed the machine.

The restore is now sampled on animation frames of the streaming bubble only. A real multi-frame replay would still fail; an intra-frame commit that the user never sees would not. The assertion also requires at least one painted sample to contain the settled background text, so a silent no-op observer cannot pass.

Fixes#3061

Verification

  • npx biome check apps/desktop/e2e/streaming-remount.spec.ts — clean
  • Isolated: playwright test e2e/streaming-remount.spec.ts --grep "accumulated while away"pass
  • Whole file after the change: 3/3, 3/3, plus one earlier whole-file run where this case still passed and an unrelated sibling (keeps a completed reply after an interrupted turn) timed out waiting for 停止 (pre-existing load flake, not this observer)
  • Before the change, this machine did not reproduce the original body-observer failure in 5 whole-file runs; the fix follows the diagnosis in the issue (observation too coarse), not a local red reproduction of that exact assertion

Not run: full desktop e2e suite, npm --workspace @maka/desktop test (no production code change)

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope:

Grok authored the test change and this PR description.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

The background-restore e2e watched document.body with a MutationObserver,
so every shell mutation during remount recorded .maka-bubble-streaming
textContent — including React commit intermediates that never painted.
That made the "no partial catch-up" assertion depend on compositor load:
it passed in isolation and failed when the rest of the file had already
warmed the machine.
Sample the bubble on animation frames instead, and require that at least
one painted sample contains the settled background text.
Fixesapache#3061
Generated-by: Grok
@coderabbitai

coderabbitaiBot commented Aug 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7cd6910e-da76-4235-9bf8-2171be824736

📥 Commits

Reviewing files that changed from the base of the PR and between 1339419 and 81add45.

📒 Files selected for processing (1)
  • apps/desktop/e2e/streaming-remount.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/desktop/e2e/streaming-remount.spec.ts

Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.


📝 Walkthrough

Summary

  • Fixes intermittent failures in the desktop background-restore E2E test.
  • Samples .maka-bubble-streaming on animation frames instead of all document.body mutations.
  • Avoids recording React commit intermediates that never paint.
  • Confirms that a painted sample contains the settled background text.
  • Confirms that no painted sample contains partially accumulated background output.

Design assessment

The change extends the existing E2E test. It does not create a parallel production path or change public contracts.

The frame sampler, sample deduplication, animation tracking, stoppable lifecycle, and next-frame stop are necessary to observe settled painted output reliably. No safe deletion or simplification is evident without weakening regression coverage.

Validation

  • Biome checks were run.
  • Repeated isolated and whole-file Playwright runs were performed.
  • The full desktop E2E suite and desktop unit tests were not run because production code did not change.
  • Required-check status remains unverified because direct current diff and status evidence is unavailable.

Review-relevant risks

No protected-area effect was identified in the current diff.

Walkthrough

The background-restore end-to-end test replaces document-wide mutation observation with a stoppable requestAnimationFrame sampler. It records distinct streaming text, tracks active animations, and stops sampling after the next animation frame.

Changes

Streaming restore test

Layer / File(s)Summary
Frame-based output sampling
apps/desktop/e2e/streaming-remount.spec.ts
The test samples the streaming bubble on animation frames, deduplicates text, tracks unfinished animations, waits for one final frame, stops the sampler, and validates restored output.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk:⚪ Minimal · up to 81add

This localized test-only change adjusts how remount output is sampled without changing product behavior, and no actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly describes the main change: sampling desktop remount restoration on painted animation frames.
Description check✅ PassedThe description follows the template, explains the problem and solution, lists verification, documents AI use, and records checklist decisions.
Linked Issues check✅ PassedThe test changes address issue #3061 by sampling painted frames and requiring settled output without partial catch-up states.
Out of Scope Changes check✅ PassedThe changes are limited to the related desktop streaming-remount E2E test and its observation logic.
Ai Use Disclosure✅ PassedThe PR selects generative tooling and names Grok with scope. Both introduced commits contain standalone Generated-by: Grok trailers.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: cc9670e7-a884-4981-8b6d-710594b20c29

📥 Commits

Reviewing files that changed from the base of the PR and between 9fbdc99 and 1339419.

📒 Files selected for processing (1)
  • apps/desktop/e2e/streaming-remount.spec.ts

Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.

Comment threadapps/desktop/e2e/streaming-remount.spec.ts
toContainText observes the DOM, not the rAF sampler. Stopping
synchronously could drop the settled paint if it landed after the last
queued sample. Stop from the next animation frame so that sample records
first.
Generated-by: Grok
@Astro-Han

Copy link
Copy Markdown
Contributor

Fast-path merge

This change qualifies for the self-merge fast path: it is low impact and easy to reverse (test-only change sampling the remount restore on painted animation frames instead of every DOM mutation), does not touch the protected areas (Runtime Host execution authority, @maka/eval semantics, public Eval CLI, security, licensing, releases, governance), and passes all required checks.

The latest head was independently reviewed by a read-only subagent (ollama-cloud/deepseek-v4-flash:high) with no open P0/P1/P2 findings. Human contributor @yuhan reviewed the final diff and chose the fast path.

@Astro-Han
Astro-Han merged commit 84a579d into apache:mainAug 16, 2026
12 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

streaming-remount background-restore test is flaky on macOS

2 participants

@1625567290@Astro-Han