Skip to content

fix(desktop): prevent collapsed workbar flash - #3794

Merged
Astro-Han merged 1 commit into
apache:mainfrom
hqhq1025:codex/fix-workbar-first-send-flash
Aug 26, 2026
Merged

fix(desktop): prevent collapsed workbar flash#3794
Astro-Han merged 1 commit into
apache:mainfrom
hqhq1025:codex/fix-workbar-first-send-flash

Conversation

@hqhq1025

Copy link
Copy Markdown
Contributor

Summary

  • Keep the lazy Workbar fallback aligned with the resolved surface visibility state.
  • Do not render a right-side loading panel when the Workbar is collapsed during the first send.
  • Preserve loading feedback for genuinely open right and bottom panels.
  • Add Electron regression coverage that records transient panel visibility across DOM mutations.

Verification

  • Red test on origin/main: the new Electron test observed visibleRightWorkbar === true during the first send.
  • npm --workspace @maka/ui run build
  • npx tsc -p apps/desktop/tsconfig.renderer.json --noEmit --pretty false
  • npx biome lint apps/desktop/src/renderer/features/workbar/ui/workbar-host.tsx apps/desktop/e2e/session-workbar.spec.ts
  • npm --workspace @maka/desktop run build:renderer
  • node --test apps/desktop/dist/main/__tests__/workbar-boundary.test.js apps/desktop/dist/main/__tests__/workbar-model.test.js (14/14 passed)
  • The targeted Electron regression passed three consecutive runs.
  • Before/after screenshots are attached in the visual evidence comment below.

Root cause

Creating the first session mounts WorkbarHost while the lazy WorkbarSurface is still resolving. The old Suspense fallback always rendered a visible 480 px right panel, even when rightCollapsed was true. Once the real surface loaded, its collapsed state hid that panel, producing the brief flash. The fallback now derives its rendered placements from the same hidden, rightCollapsed, and bottomOpen state as the resolved surface.

AI use

Select exactly one:

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

Tool(s) and scope: Codex investigated the render path, implemented the fix and regression test, and ran the verification described above.

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

@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

Visual evidence

Same viewport, first-send state, and fixture backend.

Before

The collapsed Workbar Suspense fallback temporarily occupies the right side.

Before: right Workbar loading panel flashes during first send

After

The collapsed Workbar remains absent while the lazy surface resolves.

After: main chat remains full width during first send

@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

CI note: the current failure occurs before the desktop build and is inherited from main: packages/runtime-host/src/__tests__/execution-host-queue.test.ts still calls the removed queryTurn, stopTurn, and startTurn helpers. The focused fix is already open as #3792. This Workbar PR does not duplicate that unrelated runtime-host change; CI should be rerun after #3792 lands.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I reviewed this head and found a blocking issue.

[P1] Claimed queue-test migration not present — build fails

execution-host-queue.test.ts:248/249/271 still references queryTurn/stopTurn/startTurn missing on RuntimeHostConnection. Hosted test is red.

简体中文存在测试迁移缺失阻断。

@hqhq1025
hqhq1025force-pushed the codex/fix-workbar-first-send-flash branch from 9114804 to 5f38f4eCompareAugust 25, 2026 13:15
@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

Rebased onto current main after #3796 merged. This removes the inherited Runtime Host compile failure noted in the earlier review; no unrelated queue-test migration is needed in this Workbar PR.

Local verification on 5f38f4e4c:

  • clean npm ci
  • full Desktop workspace dependency build
  • full Desktop build
  • Electron regression: a collapsed workbar never flashes during the first send (1 passed)
  • git diff --check origin/main...HEAD

Please re-review the refreshed head.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Update on 5f38f4e:

The prior P1 (queue-test still references removed RuntimeHostConnection methods) is now closed. The new head has rebased onto main with #3796's fix, execution-host-queue.test.ts no longer references queryTurn/startTurn/stopTurn (grep 0), and hosted test is now pass (run 32852297860).

The workbar-host fallback logic remains correct (no P0-P3) and is now merge-ready pending human decision.

简体中文该头 P1 已随 rebase 闭合。

@M4n5ter
M4n5terforce-pushed the codex/fix-workbar-first-send-flash branch from 5f38f4e to 4fc642aCompareAugust 26, 2026 09:07
Make the lazy Workbar fallback mirror the resolved surface visibility so creating a session cannot briefly open a collapsed panel. Add Electron coverage that records transient right-panel visibility during the first send.
Generated-by: Codex
@M4n5ter
M4n5terforce-pushed the codex/fix-workbar-first-send-flash branch from 4fc642a to 5b9627dCompareAugust 26, 2026 09:53

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The flash is real and the fix is at the right place: the lazy fallback was rendering a right-placement card unconditionally, so a collapsed workbar still flashed a loading panel while WorkbarSurface resolved on the first send.

I checked that the fix does not trade one flash for another: .maka-workbar-workspace-contents is display: contents, the cards position by grid-area, and [data-collapsed] is display: none — so omitting a card and rendering it collapsed are equivalent for layout, and nothing shifts when the surface resolves. The new e2e was verified red on main, which is the part that matters most for a one-frame regression.

Approving. One P3 and one note inline, neither blocking.

AI use: Claude Code assisted with source investigation; the analysis and conclusions are my own.

简体中文

闪烁是真实的,修复位置也对:懒加载的 fallback 无条件渲染了一张 right placement 的卡片,所以首次发送时即使工作栏是收起的,也会闪出 loading 面板。

我确认了这个修复没有用一种闪烁换另一种:.maka-workbar-workspace-contentsdisplay: contents,卡片靠 grid-area 定位,[data-collapsed]display: none——所以「不渲染」和「渲染后折叠」在布局上等价,surface 解析完成时不会发生位移。新增的 e2e 在 main 上验证为红,对一帧级别的回归来说这是最关键的一点。

Approve。行内一条 P3、一条说明,都不阻塞。

bottomOpen: boolean;
}) {
const copy = getShellCopy(useUiLocale()).app;
if (props.hidden || (props.rightCollapsed && !props.bottomOpen)) return null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P3] This re-derives a rule that already has an owner. workbar-surface.tsx:742 computes the same fact as

constvisible=!props.hidden&&(placement==='right' ? !props.rightCollapsed : props.bottomOpen);

and the fallback now expresses it a second time, in a different shape. The two agree today, which is why this is P3 — but a fallback drifting from the resolved surface is exactly the bug this PR is fixing, so leaving a second copy of the rule behind reopens the same seam.

The surface also renders both cards always and hides the invisible one with data-collapsed, while the fallback omits it instead. Same pixels (the CSS makes them equivalent), different DOM. Exporting one predicate and letting the fallback mirror the surface's structure collapses both differences, and the hidden || (rightCollapsed && !bottomOpen) early return then falls out — it is already implied by the placement list being empty.

watch.visibleRightWorkbar = true;
}
};
const observer = new MutationObserver(inspect);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The observer detects by sampling rather than by record: inspect() re-queries the live DOM, so a card added and removed inside one mutation batch leaves records behind but nothing for the query to find. It caught the real regression — you verified it red on main — so this is not a problem today, just the part that would quietly stop catching things. Reading the added nodes out of records would make the detection independent of how fast the flash is.

@Astro-Han
Astro-Han merged commit 0a3390c into apache:mainAug 26, 2026
2 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.

2 participants

@hqhq1025@Astro-Han
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
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;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
fix(desktop): prevent collapsed workbar flash by hqhq1025 · Pull Request #3794 · apache/maka · GitHub
Skip to content

fix(desktop): prevent collapsed workbar flash - #3794

Merged
Astro-Han merged 1 commit into
apache:mainfrom
hqhq1025:codex/fix-workbar-first-send-flash
Aug 26, 2026
Merged

fix(desktop): prevent collapsed workbar flash#3794
Astro-Han merged 1 commit into
apache:mainfrom
hqhq1025:codex/fix-workbar-first-send-flash

Conversation

@hqhq1025

Copy link
Copy Markdown
Contributor

Summary

  • Keep the lazy Workbar fallback aligned with the resolved surface visibility state.
  • Do not render a right-side loading panel when the Workbar is collapsed during the first send.
  • Preserve loading feedback for genuinely open right and bottom panels.
  • Add Electron regression coverage that records transient panel visibility across DOM mutations.

Verification

  • Red test on origin/main: the new Electron test observed visibleRightWorkbar === true during the first send.
  • npm --workspace @maka/ui run build
  • npx tsc -p apps/desktop/tsconfig.renderer.json --noEmit --pretty false
  • npx biome lint apps/desktop/src/renderer/features/workbar/ui/workbar-host.tsx apps/desktop/e2e/session-workbar.spec.ts
  • npm --workspace @maka/desktop run build:renderer
  • node --test apps/desktop/dist/main/__tests__/workbar-boundary.test.js apps/desktop/dist/main/__tests__/workbar-model.test.js (14/14 passed)
  • The targeted Electron regression passed three consecutive runs.
  • Before/after screenshots are attached in the visual evidence comment below.

Root cause

Creating the first session mounts WorkbarHost while the lazy WorkbarSurface is still resolving. The old Suspense fallback always rendered a visible 480 px right panel, even when rightCollapsed was true. Once the real surface loaded, its collapsed state hid that panel, producing the brief flash. The fallback now derives its rendered placements from the same hidden, rightCollapsed, and bottomOpen state as the resolved surface.

AI use

Select exactly one:

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

Tool(s) and scope: Codex investigated the render path, implemented the fix and regression test, and ran the verification described above.

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

@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

Visual evidence

Same viewport, first-send state, and fixture backend.

Before

The collapsed Workbar Suspense fallback temporarily occupies the right side.

Before: right Workbar loading panel flashes during first send

After

The collapsed Workbar remains absent while the lazy surface resolves.

After: main chat remains full width during first send

@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

CI note: the current failure occurs before the desktop build and is inherited from main: packages/runtime-host/src/__tests__/execution-host-queue.test.ts still calls the removed queryTurn, stopTurn, and startTurn helpers. The focused fix is already open as #3792. This Workbar PR does not duplicate that unrelated runtime-host change; CI should be rerun after #3792 lands.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I reviewed this head and found a blocking issue.

[P1] Claimed queue-test migration not present — build fails

execution-host-queue.test.ts:248/249/271 still references queryTurn/stopTurn/startTurn missing on RuntimeHostConnection. Hosted test is red.

简体中文存在测试迁移缺失阻断。

@hqhq1025
hqhq1025force-pushed the codex/fix-workbar-first-send-flash branch from 9114804 to 5f38f4eCompareAugust 25, 2026 13:15
@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

Rebased onto current main after #3796 merged. This removes the inherited Runtime Host compile failure noted in the earlier review; no unrelated queue-test migration is needed in this Workbar PR.

Local verification on 5f38f4e4c:

  • clean npm ci
  • full Desktop workspace dependency build
  • full Desktop build
  • Electron regression: a collapsed workbar never flashes during the first send (1 passed)
  • git diff --check origin/main...HEAD

Please re-review the refreshed head.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Update on 5f38f4e:

The prior P1 (queue-test still references removed RuntimeHostConnection methods) is now closed. The new head has rebased onto main with #3796's fix, execution-host-queue.test.ts no longer references queryTurn/startTurn/stopTurn (grep 0), and hosted test is now pass (run 32852297860).

The workbar-host fallback logic remains correct (no P0-P3) and is now merge-ready pending human decision.

简体中文该头 P1 已随 rebase 闭合。

@M4n5ter
M4n5terforce-pushed the codex/fix-workbar-first-send-flash branch from 5f38f4e to 4fc642aCompareAugust 26, 2026 09:07
Make the lazy Workbar fallback mirror the resolved surface visibility so creating a session cannot briefly open a collapsed panel. Add Electron coverage that records transient right-panel visibility during the first send.
Generated-by: Codex
@M4n5ter
M4n5terforce-pushed the codex/fix-workbar-first-send-flash branch from 4fc642a to 5b9627dCompareAugust 26, 2026 09:53

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The flash is real and the fix is at the right place: the lazy fallback was rendering a right-placement card unconditionally, so a collapsed workbar still flashed a loading panel while WorkbarSurface resolved on the first send.

I checked that the fix does not trade one flash for another: .maka-workbar-workspace-contents is display: contents, the cards position by grid-area, and [data-collapsed] is display: none — so omitting a card and rendering it collapsed are equivalent for layout, and nothing shifts when the surface resolves. The new e2e was verified red on main, which is the part that matters most for a one-frame regression.

Approving. One P3 and one note inline, neither blocking.

AI use: Claude Code assisted with source investigation; the analysis and conclusions are my own.

简体中文

闪烁是真实的,修复位置也对:懒加载的 fallback 无条件渲染了一张 right placement 的卡片,所以首次发送时即使工作栏是收起的,也会闪出 loading 面板。

我确认了这个修复没有用一种闪烁换另一种:.maka-workbar-workspace-contentsdisplay: contents,卡片靠 grid-area 定位,[data-collapsed]display: none——所以「不渲染」和「渲染后折叠」在布局上等价,surface 解析完成时不会发生位移。新增的 e2e 在 main 上验证为红,对一帧级别的回归来说这是最关键的一点。

Approve。行内一条 P3、一条说明,都不阻塞。

bottomOpen: boolean;
}) {
const copy = getShellCopy(useUiLocale()).app;
if (props.hidden || (props.rightCollapsed && !props.bottomOpen)) return null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P3] This re-derives a rule that already has an owner. workbar-surface.tsx:742 computes the same fact as

constvisible=!props.hidden&&(placement==='right' ? !props.rightCollapsed : props.bottomOpen);

and the fallback now expresses it a second time, in a different shape. The two agree today, which is why this is P3 — but a fallback drifting from the resolved surface is exactly the bug this PR is fixing, so leaving a second copy of the rule behind reopens the same seam.

The surface also renders both cards always and hides the invisible one with data-collapsed, while the fallback omits it instead. Same pixels (the CSS makes them equivalent), different DOM. Exporting one predicate and letting the fallback mirror the surface's structure collapses both differences, and the hidden || (rightCollapsed && !bottomOpen) early return then falls out — it is already implied by the placement list being empty.

watch.visibleRightWorkbar = true;
}
};
const observer = new MutationObserver(inspect);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The observer detects by sampling rather than by record: inspect() re-queries the live DOM, so a card added and removed inside one mutation batch leaves records behind but nothing for the query to find. It caught the real regression — you verified it red on main — so this is not a problem today, just the part that would quietly stop catching things. Reading the added nodes out of records would make the detection independent of how fast the flash is.

@Astro-Han
Astro-Han merged commit 0a3390c into apache:mainAug 26, 2026
2 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.

2 participants

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

fix(desktop): prevent collapsed workbar flash - #3794

Merged
Astro-Han merged 1 commit into
apache:mainfrom
hqhq1025:codex/fix-workbar-first-send-flash
Aug 26, 2026
Merged

fix(desktop): prevent collapsed workbar flash#3794
Astro-Han merged 1 commit into
apache:mainfrom
hqhq1025:codex/fix-workbar-first-send-flash

Conversation

@hqhq1025

Copy link
Copy Markdown
Contributor

Summary

  • Keep the lazy Workbar fallback aligned with the resolved surface visibility state.
  • Do not render a right-side loading panel when the Workbar is collapsed during the first send.
  • Preserve loading feedback for genuinely open right and bottom panels.
  • Add Electron regression coverage that records transient panel visibility across DOM mutations.

Verification

  • Red test on origin/main: the new Electron test observed visibleRightWorkbar === true during the first send.
  • npm --workspace @maka/ui run build
  • npx tsc -p apps/desktop/tsconfig.renderer.json --noEmit --pretty false
  • npx biome lint apps/desktop/src/renderer/features/workbar/ui/workbar-host.tsx apps/desktop/e2e/session-workbar.spec.ts
  • npm --workspace @maka/desktop run build:renderer
  • node --test apps/desktop/dist/main/__tests__/workbar-boundary.test.js apps/desktop/dist/main/__tests__/workbar-model.test.js (14/14 passed)
  • The targeted Electron regression passed three consecutive runs.
  • Before/after screenshots are attached in the visual evidence comment below.

Root cause

Creating the first session mounts WorkbarHost while the lazy WorkbarSurface is still resolving. The old Suspense fallback always rendered a visible 480 px right panel, even when rightCollapsed was true. Once the real surface loaded, its collapsed state hid that panel, producing the brief flash. The fallback now derives its rendered placements from the same hidden, rightCollapsed, and bottomOpen state as the resolved surface.

AI use

Select exactly one:

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

Tool(s) and scope: Codex investigated the render path, implemented the fix and regression test, and ran the verification described above.

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

@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

Visual evidence

Same viewport, first-send state, and fixture backend.

Before

The collapsed Workbar Suspense fallback temporarily occupies the right side.

Before: right Workbar loading panel flashes during first send

After

The collapsed Workbar remains absent while the lazy surface resolves.

After: main chat remains full width during first send

@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

CI note: the current failure occurs before the desktop build and is inherited from main: packages/runtime-host/src/__tests__/execution-host-queue.test.ts still calls the removed queryTurn, stopTurn, and startTurn helpers. The focused fix is already open as #3792. This Workbar PR does not duplicate that unrelated runtime-host change; CI should be rerun after #3792 lands.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I reviewed this head and found a blocking issue.

[P1] Claimed queue-test migration not present — build fails

execution-host-queue.test.ts:248/249/271 still references queryTurn/stopTurn/startTurn missing on RuntimeHostConnection. Hosted test is red.

简体中文存在测试迁移缺失阻断。

@hqhq1025
hqhq1025force-pushed the codex/fix-workbar-first-send-flash branch from 9114804 to 5f38f4eCompareAugust 25, 2026 13:15
@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

Rebased onto current main after #3796 merged. This removes the inherited Runtime Host compile failure noted in the earlier review; no unrelated queue-test migration is needed in this Workbar PR.

Local verification on 5f38f4e4c:

  • clean npm ci
  • full Desktop workspace dependency build
  • full Desktop build
  • Electron regression: a collapsed workbar never flashes during the first send (1 passed)
  • git diff --check origin/main...HEAD

Please re-review the refreshed head.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Update on 5f38f4e:

The prior P1 (queue-test still references removed RuntimeHostConnection methods) is now closed. The new head has rebased onto main with #3796's fix, execution-host-queue.test.ts no longer references queryTurn/startTurn/stopTurn (grep 0), and hosted test is now pass (run 32852297860).

The workbar-host fallback logic remains correct (no P0-P3) and is now merge-ready pending human decision.

简体中文该头 P1 已随 rebase 闭合。

@M4n5ter
M4n5terforce-pushed the codex/fix-workbar-first-send-flash branch from 5f38f4e to 4fc642aCompareAugust 26, 2026 09:07
Make the lazy Workbar fallback mirror the resolved surface visibility so creating a session cannot briefly open a collapsed panel. Add Electron coverage that records transient right-panel visibility during the first send.
Generated-by: Codex
@M4n5ter
M4n5terforce-pushed the codex/fix-workbar-first-send-flash branch from 4fc642a to 5b9627dCompareAugust 26, 2026 09:53

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The flash is real and the fix is at the right place: the lazy fallback was rendering a right-placement card unconditionally, so a collapsed workbar still flashed a loading panel while WorkbarSurface resolved on the first send.

I checked that the fix does not trade one flash for another: .maka-workbar-workspace-contents is display: contents, the cards position by grid-area, and [data-collapsed] is display: none — so omitting a card and rendering it collapsed are equivalent for layout, and nothing shifts when the surface resolves. The new e2e was verified red on main, which is the part that matters most for a one-frame regression.

Approving. One P3 and one note inline, neither blocking.

AI use: Claude Code assisted with source investigation; the analysis and conclusions are my own.

简体中文

闪烁是真实的,修复位置也对:懒加载的 fallback 无条件渲染了一张 right placement 的卡片,所以首次发送时即使工作栏是收起的,也会闪出 loading 面板。

我确认了这个修复没有用一种闪烁换另一种:.maka-workbar-workspace-contentsdisplay: contents,卡片靠 grid-area 定位,[data-collapsed]display: none——所以「不渲染」和「渲染后折叠」在布局上等价,surface 解析完成时不会发生位移。新增的 e2e 在 main 上验证为红,对一帧级别的回归来说这是最关键的一点。

Approve。行内一条 P3、一条说明,都不阻塞。

bottomOpen: boolean;
}) {
const copy = getShellCopy(useUiLocale()).app;
if (props.hidden || (props.rightCollapsed && !props.bottomOpen)) return null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P3] This re-derives a rule that already has an owner. workbar-surface.tsx:742 computes the same fact as

constvisible=!props.hidden&&(placement==='right' ? !props.rightCollapsed : props.bottomOpen);

and the fallback now expresses it a second time, in a different shape. The two agree today, which is why this is P3 — but a fallback drifting from the resolved surface is exactly the bug this PR is fixing, so leaving a second copy of the rule behind reopens the same seam.

The surface also renders both cards always and hides the invisible one with data-collapsed, while the fallback omits it instead. Same pixels (the CSS makes them equivalent), different DOM. Exporting one predicate and letting the fallback mirror the surface's structure collapses both differences, and the hidden || (rightCollapsed && !bottomOpen) early return then falls out — it is already implied by the placement list being empty.

watch.visibleRightWorkbar = true;
}
};
const observer = new MutationObserver(inspect);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The observer detects by sampling rather than by record: inspect() re-queries the live DOM, so a card added and removed inside one mutation batch leaves records behind but nothing for the query to find. It caught the real regression — you verified it red on main — so this is not a problem today, just the part that would quietly stop catching things. Reading the added nodes out of records would make the detection independent of how fast the flash is.

@Astro-Han
Astro-Han merged commit 0a3390c into apache:mainAug 26, 2026
2 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.

2 participants

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

fix(desktop): prevent collapsed workbar flash - #3794

Merged
Astro-Han merged 1 commit into
apache:mainfrom
hqhq1025:codex/fix-workbar-first-send-flash
Aug 26, 2026
Merged

fix(desktop): prevent collapsed workbar flash#3794
Astro-Han merged 1 commit into
apache:mainfrom
hqhq1025:codex/fix-workbar-first-send-flash

Conversation

@hqhq1025

Copy link
Copy Markdown
Contributor

Summary

  • Keep the lazy Workbar fallback aligned with the resolved surface visibility state.
  • Do not render a right-side loading panel when the Workbar is collapsed during the first send.
  • Preserve loading feedback for genuinely open right and bottom panels.
  • Add Electron regression coverage that records transient panel visibility across DOM mutations.

Verification

  • Red test on origin/main: the new Electron test observed visibleRightWorkbar === true during the first send.
  • npm --workspace @maka/ui run build
  • npx tsc -p apps/desktop/tsconfig.renderer.json --noEmit --pretty false
  • npx biome lint apps/desktop/src/renderer/features/workbar/ui/workbar-host.tsx apps/desktop/e2e/session-workbar.spec.ts
  • npm --workspace @maka/desktop run build:renderer
  • node --test apps/desktop/dist/main/__tests__/workbar-boundary.test.js apps/desktop/dist/main/__tests__/workbar-model.test.js (14/14 passed)
  • The targeted Electron regression passed three consecutive runs.
  • Before/after screenshots are attached in the visual evidence comment below.

Root cause

Creating the first session mounts WorkbarHost while the lazy WorkbarSurface is still resolving. The old Suspense fallback always rendered a visible 480 px right panel, even when rightCollapsed was true. Once the real surface loaded, its collapsed state hid that panel, producing the brief flash. The fallback now derives its rendered placements from the same hidden, rightCollapsed, and bottomOpen state as the resolved surface.

AI use

Select exactly one:

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

Tool(s) and scope: Codex investigated the render path, implemented the fix and regression test, and ran the verification described above.

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

@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

Visual evidence

Same viewport, first-send state, and fixture backend.

Before

The collapsed Workbar Suspense fallback temporarily occupies the right side.

Before: right Workbar loading panel flashes during first send

After

The collapsed Workbar remains absent while the lazy surface resolves.

After: main chat remains full width during first send

@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

CI note: the current failure occurs before the desktop build and is inherited from main: packages/runtime-host/src/__tests__/execution-host-queue.test.ts still calls the removed queryTurn, stopTurn, and startTurn helpers. The focused fix is already open as #3792. This Workbar PR does not duplicate that unrelated runtime-host change; CI should be rerun after #3792 lands.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I reviewed this head and found a blocking issue.

[P1] Claimed queue-test migration not present — build fails

execution-host-queue.test.ts:248/249/271 still references queryTurn/stopTurn/startTurn missing on RuntimeHostConnection. Hosted test is red.

简体中文存在测试迁移缺失阻断。

@hqhq1025
hqhq1025force-pushed the codex/fix-workbar-first-send-flash branch from 9114804 to 5f38f4eCompareAugust 25, 2026 13:15
@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

Rebased onto current main after #3796 merged. This removes the inherited Runtime Host compile failure noted in the earlier review; no unrelated queue-test migration is needed in this Workbar PR.

Local verification on 5f38f4e4c:

  • clean npm ci
  • full Desktop workspace dependency build
  • full Desktop build
  • Electron regression: a collapsed workbar never flashes during the first send (1 passed)
  • git diff --check origin/main...HEAD

Please re-review the refreshed head.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Update on 5f38f4e:

The prior P1 (queue-test still references removed RuntimeHostConnection methods) is now closed. The new head has rebased onto main with #3796's fix, execution-host-queue.test.ts no longer references queryTurn/startTurn/stopTurn (grep 0), and hosted test is now pass (run 32852297860).

The workbar-host fallback logic remains correct (no P0-P3) and is now merge-ready pending human decision.

简体中文该头 P1 已随 rebase 闭合。

@M4n5ter
M4n5terforce-pushed the codex/fix-workbar-first-send-flash branch from 5f38f4e to 4fc642aCompareAugust 26, 2026 09:07
Make the lazy Workbar fallback mirror the resolved surface visibility so creating a session cannot briefly open a collapsed panel. Add Electron coverage that records transient right-panel visibility during the first send.
Generated-by: Codex
@M4n5ter
M4n5terforce-pushed the codex/fix-workbar-first-send-flash branch from 4fc642a to 5b9627dCompareAugust 26, 2026 09:53

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The flash is real and the fix is at the right place: the lazy fallback was rendering a right-placement card unconditionally, so a collapsed workbar still flashed a loading panel while WorkbarSurface resolved on the first send.

I checked that the fix does not trade one flash for another: .maka-workbar-workspace-contents is display: contents, the cards position by grid-area, and [data-collapsed] is display: none — so omitting a card and rendering it collapsed are equivalent for layout, and nothing shifts when the surface resolves. The new e2e was verified red on main, which is the part that matters most for a one-frame regression.

Approving. One P3 and one note inline, neither blocking.

AI use: Claude Code assisted with source investigation; the analysis and conclusions are my own.

简体中文

闪烁是真实的,修复位置也对:懒加载的 fallback 无条件渲染了一张 right placement 的卡片,所以首次发送时即使工作栏是收起的,也会闪出 loading 面板。

我确认了这个修复没有用一种闪烁换另一种:.maka-workbar-workspace-contentsdisplay: contents,卡片靠 grid-area 定位,[data-collapsed]display: none——所以「不渲染」和「渲染后折叠」在布局上等价,surface 解析完成时不会发生位移。新增的 e2e 在 main 上验证为红,对一帧级别的回归来说这是最关键的一点。

Approve。行内一条 P3、一条说明,都不阻塞。

bottomOpen: boolean;
}) {
const copy = getShellCopy(useUiLocale()).app;
if (props.hidden || (props.rightCollapsed && !props.bottomOpen)) return null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P3] This re-derives a rule that already has an owner. workbar-surface.tsx:742 computes the same fact as

constvisible=!props.hidden&&(placement==='right' ? !props.rightCollapsed : props.bottomOpen);

and the fallback now expresses it a second time, in a different shape. The two agree today, which is why this is P3 — but a fallback drifting from the resolved surface is exactly the bug this PR is fixing, so leaving a second copy of the rule behind reopens the same seam.

The surface also renders both cards always and hides the invisible one with data-collapsed, while the fallback omits it instead. Same pixels (the CSS makes them equivalent), different DOM. Exporting one predicate and letting the fallback mirror the surface's structure collapses both differences, and the hidden || (rightCollapsed && !bottomOpen) early return then falls out — it is already implied by the placement list being empty.

watch.visibleRightWorkbar = true;
}
};
const observer = new MutationObserver(inspect);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The observer detects by sampling rather than by record: inspect() re-queries the live DOM, so a card added and removed inside one mutation batch leaves records behind but nothing for the query to find. It caught the real regression — you verified it red on main — so this is not a problem today, just the part that would quietly stop catching things. Reading the added nodes out of records would make the detection independent of how fast the flash is.

@Astro-Han
Astro-Han merged commit 0a3390c into apache:mainAug 26, 2026
2 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.

2 participants

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

fix(desktop): prevent collapsed workbar flash - #3794

Merged
Astro-Han merged 1 commit into
apache:mainfrom
hqhq1025:codex/fix-workbar-first-send-flash
Aug 26, 2026
Merged

fix(desktop): prevent collapsed workbar flash#3794
Astro-Han merged 1 commit into
apache:mainfrom
hqhq1025:codex/fix-workbar-first-send-flash

Conversation

@hqhq1025

Copy link
Copy Markdown
Contributor

Summary

  • Keep the lazy Workbar fallback aligned with the resolved surface visibility state.
  • Do not render a right-side loading panel when the Workbar is collapsed during the first send.
  • Preserve loading feedback for genuinely open right and bottom panels.
  • Add Electron regression coverage that records transient panel visibility across DOM mutations.

Verification

  • Red test on origin/main: the new Electron test observed visibleRightWorkbar === true during the first send.
  • npm --workspace @maka/ui run build
  • npx tsc -p apps/desktop/tsconfig.renderer.json --noEmit --pretty false
  • npx biome lint apps/desktop/src/renderer/features/workbar/ui/workbar-host.tsx apps/desktop/e2e/session-workbar.spec.ts
  • npm --workspace @maka/desktop run build:renderer
  • node --test apps/desktop/dist/main/__tests__/workbar-boundary.test.js apps/desktop/dist/main/__tests__/workbar-model.test.js (14/14 passed)
  • The targeted Electron regression passed three consecutive runs.
  • Before/after screenshots are attached in the visual evidence comment below.

Root cause

Creating the first session mounts WorkbarHost while the lazy WorkbarSurface is still resolving. The old Suspense fallback always rendered a visible 480 px right panel, even when rightCollapsed was true. Once the real surface loaded, its collapsed state hid that panel, producing the brief flash. The fallback now derives its rendered placements from the same hidden, rightCollapsed, and bottomOpen state as the resolved surface.

AI use

Select exactly one:

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

Tool(s) and scope: Codex investigated the render path, implemented the fix and regression test, and ran the verification described above.

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

@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

Visual evidence

Same viewport, first-send state, and fixture backend.

Before

The collapsed Workbar Suspense fallback temporarily occupies the right side.

Before: right Workbar loading panel flashes during first send

After

The collapsed Workbar remains absent while the lazy surface resolves.

After: main chat remains full width during first send

@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

CI note: the current failure occurs before the desktop build and is inherited from main: packages/runtime-host/src/__tests__/execution-host-queue.test.ts still calls the removed queryTurn, stopTurn, and startTurn helpers. The focused fix is already open as #3792. This Workbar PR does not duplicate that unrelated runtime-host change; CI should be rerun after #3792 lands.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I reviewed this head and found a blocking issue.

[P1] Claimed queue-test migration not present — build fails

execution-host-queue.test.ts:248/249/271 still references queryTurn/stopTurn/startTurn missing on RuntimeHostConnection. Hosted test is red.

简体中文存在测试迁移缺失阻断。

@hqhq1025
hqhq1025force-pushed the codex/fix-workbar-first-send-flash branch from 9114804 to 5f38f4eCompareAugust 25, 2026 13:15
@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

Rebased onto current main after #3796 merged. This removes the inherited Runtime Host compile failure noted in the earlier review; no unrelated queue-test migration is needed in this Workbar PR.

Local verification on 5f38f4e4c:

  • clean npm ci
  • full Desktop workspace dependency build
  • full Desktop build
  • Electron regression: a collapsed workbar never flashes during the first send (1 passed)
  • git diff --check origin/main...HEAD

Please re-review the refreshed head.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Update on 5f38f4e:

The prior P1 (queue-test still references removed RuntimeHostConnection methods) is now closed. The new head has rebased onto main with #3796's fix, execution-host-queue.test.ts no longer references queryTurn/startTurn/stopTurn (grep 0), and hosted test is now pass (run 32852297860).

The workbar-host fallback logic remains correct (no P0-P3) and is now merge-ready pending human decision.

简体中文该头 P1 已随 rebase 闭合。

@M4n5ter
M4n5terforce-pushed the codex/fix-workbar-first-send-flash branch from 5f38f4e to 4fc642aCompareAugust 26, 2026 09:07
Make the lazy Workbar fallback mirror the resolved surface visibility so creating a session cannot briefly open a collapsed panel. Add Electron coverage that records transient right-panel visibility during the first send.
Generated-by: Codex
@M4n5ter
M4n5terforce-pushed the codex/fix-workbar-first-send-flash branch from 4fc642a to 5b9627dCompareAugust 26, 2026 09:53

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The flash is real and the fix is at the right place: the lazy fallback was rendering a right-placement card unconditionally, so a collapsed workbar still flashed a loading panel while WorkbarSurface resolved on the first send.

I checked that the fix does not trade one flash for another: .maka-workbar-workspace-contents is display: contents, the cards position by grid-area, and [data-collapsed] is display: none — so omitting a card and rendering it collapsed are equivalent for layout, and nothing shifts when the surface resolves. The new e2e was verified red on main, which is the part that matters most for a one-frame regression.

Approving. One P3 and one note inline, neither blocking.

AI use: Claude Code assisted with source investigation; the analysis and conclusions are my own.

简体中文

闪烁是真实的,修复位置也对:懒加载的 fallback 无条件渲染了一张 right placement 的卡片,所以首次发送时即使工作栏是收起的,也会闪出 loading 面板。

我确认了这个修复没有用一种闪烁换另一种:.maka-workbar-workspace-contentsdisplay: contents,卡片靠 grid-area 定位,[data-collapsed]display: none——所以「不渲染」和「渲染后折叠」在布局上等价,surface 解析完成时不会发生位移。新增的 e2e 在 main 上验证为红,对一帧级别的回归来说这是最关键的一点。

Approve。行内一条 P3、一条说明,都不阻塞。

bottomOpen: boolean;
}) {
const copy = getShellCopy(useUiLocale()).app;
if (props.hidden || (props.rightCollapsed && !props.bottomOpen)) return null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P3] This re-derives a rule that already has an owner. workbar-surface.tsx:742 computes the same fact as

constvisible=!props.hidden&&(placement==='right' ? !props.rightCollapsed : props.bottomOpen);

and the fallback now expresses it a second time, in a different shape. The two agree today, which is why this is P3 — but a fallback drifting from the resolved surface is exactly the bug this PR is fixing, so leaving a second copy of the rule behind reopens the same seam.

The surface also renders both cards always and hides the invisible one with data-collapsed, while the fallback omits it instead. Same pixels (the CSS makes them equivalent), different DOM. Exporting one predicate and letting the fallback mirror the surface's structure collapses both differences, and the hidden || (rightCollapsed && !bottomOpen) early return then falls out — it is already implied by the placement list being empty.

watch.visibleRightWorkbar = true;
}
};
const observer = new MutationObserver(inspect);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The observer detects by sampling rather than by record: inspect() re-queries the live DOM, so a card added and removed inside one mutation batch leaves records behind but nothing for the query to find. It caught the real regression — you verified it red on main — so this is not a problem today, just the part that would quietly stop catching things. Reading the added nodes out of records would make the detection independent of how fast the flash is.

@Astro-Han
Astro-Han merged commit 0a3390c into apache:mainAug 26, 2026
2 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.

2 participants

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

fix(desktop): prevent collapsed workbar flash - #3794

Merged
Astro-Han merged 1 commit into
apache:mainfrom
hqhq1025:codex/fix-workbar-first-send-flash
Aug 26, 2026
Merged

fix(desktop): prevent collapsed workbar flash#3794
Astro-Han merged 1 commit into
apache:mainfrom
hqhq1025:codex/fix-workbar-first-send-flash

Conversation

@hqhq1025

Copy link
Copy Markdown
Contributor

Summary

  • Keep the lazy Workbar fallback aligned with the resolved surface visibility state.
  • Do not render a right-side loading panel when the Workbar is collapsed during the first send.
  • Preserve loading feedback for genuinely open right and bottom panels.
  • Add Electron regression coverage that records transient panel visibility across DOM mutations.

Verification

  • Red test on origin/main: the new Electron test observed visibleRightWorkbar === true during the first send.
  • npm --workspace @maka/ui run build
  • npx tsc -p apps/desktop/tsconfig.renderer.json --noEmit --pretty false
  • npx biome lint apps/desktop/src/renderer/features/workbar/ui/workbar-host.tsx apps/desktop/e2e/session-workbar.spec.ts
  • npm --workspace @maka/desktop run build:renderer
  • node --test apps/desktop/dist/main/__tests__/workbar-boundary.test.js apps/desktop/dist/main/__tests__/workbar-model.test.js (14/14 passed)
  • The targeted Electron regression passed three consecutive runs.
  • Before/after screenshots are attached in the visual evidence comment below.

Root cause

Creating the first session mounts WorkbarHost while the lazy WorkbarSurface is still resolving. The old Suspense fallback always rendered a visible 480 px right panel, even when rightCollapsed was true. Once the real surface loaded, its collapsed state hid that panel, producing the brief flash. The fallback now derives its rendered placements from the same hidden, rightCollapsed, and bottomOpen state as the resolved surface.

AI use

Select exactly one:

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

Tool(s) and scope: Codex investigated the render path, implemented the fix and regression test, and ran the verification described above.

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

@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

Visual evidence

Same viewport, first-send state, and fixture backend.

Before

The collapsed Workbar Suspense fallback temporarily occupies the right side.

Before: right Workbar loading panel flashes during first send

After

The collapsed Workbar remains absent while the lazy surface resolves.

After: main chat remains full width during first send

@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

CI note: the current failure occurs before the desktop build and is inherited from main: packages/runtime-host/src/__tests__/execution-host-queue.test.ts still calls the removed queryTurn, stopTurn, and startTurn helpers. The focused fix is already open as #3792. This Workbar PR does not duplicate that unrelated runtime-host change; CI should be rerun after #3792 lands.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I reviewed this head and found a blocking issue.

[P1] Claimed queue-test migration not present — build fails

execution-host-queue.test.ts:248/249/271 still references queryTurn/stopTurn/startTurn missing on RuntimeHostConnection. Hosted test is red.

简体中文存在测试迁移缺失阻断。

@hqhq1025
hqhq1025force-pushed the codex/fix-workbar-first-send-flash branch from 9114804 to 5f38f4eCompareAugust 25, 2026 13:15
@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

Rebased onto current main after #3796 merged. This removes the inherited Runtime Host compile failure noted in the earlier review; no unrelated queue-test migration is needed in this Workbar PR.

Local verification on 5f38f4e4c:

  • clean npm ci
  • full Desktop workspace dependency build
  • full Desktop build
  • Electron regression: a collapsed workbar never flashes during the first send (1 passed)
  • git diff --check origin/main...HEAD

Please re-review the refreshed head.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Update on 5f38f4e:

The prior P1 (queue-test still references removed RuntimeHostConnection methods) is now closed. The new head has rebased onto main with #3796's fix, execution-host-queue.test.ts no longer references queryTurn/startTurn/stopTurn (grep 0), and hosted test is now pass (run 32852297860).

The workbar-host fallback logic remains correct (no P0-P3) and is now merge-ready pending human decision.

简体中文该头 P1 已随 rebase 闭合。

@M4n5ter
M4n5terforce-pushed the codex/fix-workbar-first-send-flash branch from 5f38f4e to 4fc642aCompareAugust 26, 2026 09:07
Make the lazy Workbar fallback mirror the resolved surface visibility so creating a session cannot briefly open a collapsed panel. Add Electron coverage that records transient right-panel visibility during the first send.
Generated-by: Codex
@M4n5ter
M4n5terforce-pushed the codex/fix-workbar-first-send-flash branch from 4fc642a to 5b9627dCompareAugust 26, 2026 09:53

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The flash is real and the fix is at the right place: the lazy fallback was rendering a right-placement card unconditionally, so a collapsed workbar still flashed a loading panel while WorkbarSurface resolved on the first send.

I checked that the fix does not trade one flash for another: .maka-workbar-workspace-contents is display: contents, the cards position by grid-area, and [data-collapsed] is display: none — so omitting a card and rendering it collapsed are equivalent for layout, and nothing shifts when the surface resolves. The new e2e was verified red on main, which is the part that matters most for a one-frame regression.

Approving. One P3 and one note inline, neither blocking.

AI use: Claude Code assisted with source investigation; the analysis and conclusions are my own.

简体中文

闪烁是真实的,修复位置也对:懒加载的 fallback 无条件渲染了一张 right placement 的卡片,所以首次发送时即使工作栏是收起的,也会闪出 loading 面板。

我确认了这个修复没有用一种闪烁换另一种:.maka-workbar-workspace-contentsdisplay: contents,卡片靠 grid-area 定位,[data-collapsed]display: none——所以「不渲染」和「渲染后折叠」在布局上等价,surface 解析完成时不会发生位移。新增的 e2e 在 main 上验证为红,对一帧级别的回归来说这是最关键的一点。

Approve。行内一条 P3、一条说明,都不阻塞。

bottomOpen: boolean;
}) {
const copy = getShellCopy(useUiLocale()).app;
if (props.hidden || (props.rightCollapsed && !props.bottomOpen)) return null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P3] This re-derives a rule that already has an owner. workbar-surface.tsx:742 computes the same fact as

constvisible=!props.hidden&&(placement==='right' ? !props.rightCollapsed : props.bottomOpen);

and the fallback now expresses it a second time, in a different shape. The two agree today, which is why this is P3 — but a fallback drifting from the resolved surface is exactly the bug this PR is fixing, so leaving a second copy of the rule behind reopens the same seam.

The surface also renders both cards always and hides the invisible one with data-collapsed, while the fallback omits it instead. Same pixels (the CSS makes them equivalent), different DOM. Exporting one predicate and letting the fallback mirror the surface's structure collapses both differences, and the hidden || (rightCollapsed && !bottomOpen) early return then falls out — it is already implied by the placement list being empty.

watch.visibleRightWorkbar = true;
}
};
const observer = new MutationObserver(inspect);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The observer detects by sampling rather than by record: inspect() re-queries the live DOM, so a card added and removed inside one mutation batch leaves records behind but nothing for the query to find. It caught the real regression — you verified it red on main — so this is not a problem today, just the part that would quietly stop catching things. Reading the added nodes out of records would make the detection independent of how fast the flash is.

@Astro-Han
Astro-Han merged commit 0a3390c into apache:mainAug 26, 2026
2 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.

2 participants

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

fix(desktop): prevent collapsed workbar flash - #3794

Merged
Astro-Han merged 1 commit into
apache:mainfrom
hqhq1025:codex/fix-workbar-first-send-flash
Aug 26, 2026
Merged

fix(desktop): prevent collapsed workbar flash#3794
Astro-Han merged 1 commit into
apache:mainfrom
hqhq1025:codex/fix-workbar-first-send-flash

Conversation

@hqhq1025

Copy link
Copy Markdown
Contributor

Summary

  • Keep the lazy Workbar fallback aligned with the resolved surface visibility state.
  • Do not render a right-side loading panel when the Workbar is collapsed during the first send.
  • Preserve loading feedback for genuinely open right and bottom panels.
  • Add Electron regression coverage that records transient panel visibility across DOM mutations.

Verification

  • Red test on origin/main: the new Electron test observed visibleRightWorkbar === true during the first send.
  • npm --workspace @maka/ui run build
  • npx tsc -p apps/desktop/tsconfig.renderer.json --noEmit --pretty false
  • npx biome lint apps/desktop/src/renderer/features/workbar/ui/workbar-host.tsx apps/desktop/e2e/session-workbar.spec.ts
  • npm --workspace @maka/desktop run build:renderer
  • node --test apps/desktop/dist/main/__tests__/workbar-boundary.test.js apps/desktop/dist/main/__tests__/workbar-model.test.js (14/14 passed)
  • The targeted Electron regression passed three consecutive runs.
  • Before/after screenshots are attached in the visual evidence comment below.

Root cause

Creating the first session mounts WorkbarHost while the lazy WorkbarSurface is still resolving. The old Suspense fallback always rendered a visible 480 px right panel, even when rightCollapsed was true. Once the real surface loaded, its collapsed state hid that panel, producing the brief flash. The fallback now derives its rendered placements from the same hidden, rightCollapsed, and bottomOpen state as the resolved surface.

AI use

Select exactly one:

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

Tool(s) and scope: Codex investigated the render path, implemented the fix and regression test, and ran the verification described above.

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

@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

Visual evidence

Same viewport, first-send state, and fixture backend.

Before

The collapsed Workbar Suspense fallback temporarily occupies the right side.

Before: right Workbar loading panel flashes during first send

After

The collapsed Workbar remains absent while the lazy surface resolves.

After: main chat remains full width during first send

@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

CI note: the current failure occurs before the desktop build and is inherited from main: packages/runtime-host/src/__tests__/execution-host-queue.test.ts still calls the removed queryTurn, stopTurn, and startTurn helpers. The focused fix is already open as #3792. This Workbar PR does not duplicate that unrelated runtime-host change; CI should be rerun after #3792 lands.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I reviewed this head and found a blocking issue.

[P1] Claimed queue-test migration not present — build fails

execution-host-queue.test.ts:248/249/271 still references queryTurn/stopTurn/startTurn missing on RuntimeHostConnection. Hosted test is red.

简体中文存在测试迁移缺失阻断。

@hqhq1025
hqhq1025force-pushed the codex/fix-workbar-first-send-flash branch from 9114804 to 5f38f4eCompareAugust 25, 2026 13:15
@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

Rebased onto current main after #3796 merged. This removes the inherited Runtime Host compile failure noted in the earlier review; no unrelated queue-test migration is needed in this Workbar PR.

Local verification on 5f38f4e4c:

  • clean npm ci
  • full Desktop workspace dependency build
  • full Desktop build
  • Electron regression: a collapsed workbar never flashes during the first send (1 passed)
  • git diff --check origin/main...HEAD

Please re-review the refreshed head.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Update on 5f38f4e:

The prior P1 (queue-test still references removed RuntimeHostConnection methods) is now closed. The new head has rebased onto main with #3796's fix, execution-host-queue.test.ts no longer references queryTurn/startTurn/stopTurn (grep 0), and hosted test is now pass (run 32852297860).

The workbar-host fallback logic remains correct (no P0-P3) and is now merge-ready pending human decision.

简体中文该头 P1 已随 rebase 闭合。

@M4n5ter
M4n5terforce-pushed the codex/fix-workbar-first-send-flash branch from 5f38f4e to 4fc642aCompareAugust 26, 2026 09:07
Make the lazy Workbar fallback mirror the resolved surface visibility so creating a session cannot briefly open a collapsed panel. Add Electron coverage that records transient right-panel visibility during the first send.
Generated-by: Codex
@M4n5ter
M4n5terforce-pushed the codex/fix-workbar-first-send-flash branch from 4fc642a to 5b9627dCompareAugust 26, 2026 09:53

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The flash is real and the fix is at the right place: the lazy fallback was rendering a right-placement card unconditionally, so a collapsed workbar still flashed a loading panel while WorkbarSurface resolved on the first send.

I checked that the fix does not trade one flash for another: .maka-workbar-workspace-contents is display: contents, the cards position by grid-area, and [data-collapsed] is display: none — so omitting a card and rendering it collapsed are equivalent for layout, and nothing shifts when the surface resolves. The new e2e was verified red on main, which is the part that matters most for a one-frame regression.

Approving. One P3 and one note inline, neither blocking.

AI use: Claude Code assisted with source investigation; the analysis and conclusions are my own.

简体中文

闪烁是真实的,修复位置也对:懒加载的 fallback 无条件渲染了一张 right placement 的卡片,所以首次发送时即使工作栏是收起的,也会闪出 loading 面板。

我确认了这个修复没有用一种闪烁换另一种:.maka-workbar-workspace-contentsdisplay: contents,卡片靠 grid-area 定位,[data-collapsed]display: none——所以「不渲染」和「渲染后折叠」在布局上等价,surface 解析完成时不会发生位移。新增的 e2e 在 main 上验证为红,对一帧级别的回归来说这是最关键的一点。

Approve。行内一条 P3、一条说明,都不阻塞。

bottomOpen: boolean;
}) {
const copy = getShellCopy(useUiLocale()).app;
if (props.hidden || (props.rightCollapsed && !props.bottomOpen)) return null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P3] This re-derives a rule that already has an owner. workbar-surface.tsx:742 computes the same fact as

constvisible=!props.hidden&&(placement==='right' ? !props.rightCollapsed : props.bottomOpen);

and the fallback now expresses it a second time, in a different shape. The two agree today, which is why this is P3 — but a fallback drifting from the resolved surface is exactly the bug this PR is fixing, so leaving a second copy of the rule behind reopens the same seam.

The surface also renders both cards always and hides the invisible one with data-collapsed, while the fallback omits it instead. Same pixels (the CSS makes them equivalent), different DOM. Exporting one predicate and letting the fallback mirror the surface's structure collapses both differences, and the hidden || (rightCollapsed && !bottomOpen) early return then falls out — it is already implied by the placement list being empty.

watch.visibleRightWorkbar = true;
}
};
const observer = new MutationObserver(inspect);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The observer detects by sampling rather than by record: inspect() re-queries the live DOM, so a card added and removed inside one mutation batch leaves records behind but nothing for the query to find. It caught the real regression — you verified it red on main — so this is not a problem today, just the part that would quietly stop catching things. Reading the added nodes out of records would make the detection independent of how fast the flash is.

@Astro-Han
Astro-Han merged commit 0a3390c into apache:mainAug 26, 2026
2 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.

2 participants

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

fix(desktop): prevent collapsed workbar flash - #3794

Merged
Astro-Han merged 1 commit into
apache:mainfrom
hqhq1025:codex/fix-workbar-first-send-flash
Aug 26, 2026
Merged

fix(desktop): prevent collapsed workbar flash#3794
Astro-Han merged 1 commit into
apache:mainfrom
hqhq1025:codex/fix-workbar-first-send-flash

Conversation

@hqhq1025

Copy link
Copy Markdown
Contributor

Summary

  • Keep the lazy Workbar fallback aligned with the resolved surface visibility state.
  • Do not render a right-side loading panel when the Workbar is collapsed during the first send.
  • Preserve loading feedback for genuinely open right and bottom panels.
  • Add Electron regression coverage that records transient panel visibility across DOM mutations.

Verification

  • Red test on origin/main: the new Electron test observed visibleRightWorkbar === true during the first send.
  • npm --workspace @maka/ui run build
  • npx tsc -p apps/desktop/tsconfig.renderer.json --noEmit --pretty false
  • npx biome lint apps/desktop/src/renderer/features/workbar/ui/workbar-host.tsx apps/desktop/e2e/session-workbar.spec.ts
  • npm --workspace @maka/desktop run build:renderer
  • node --test apps/desktop/dist/main/__tests__/workbar-boundary.test.js apps/desktop/dist/main/__tests__/workbar-model.test.js (14/14 passed)
  • The targeted Electron regression passed three consecutive runs.
  • Before/after screenshots are attached in the visual evidence comment below.

Root cause

Creating the first session mounts WorkbarHost while the lazy WorkbarSurface is still resolving. The old Suspense fallback always rendered a visible 480 px right panel, even when rightCollapsed was true. Once the real surface loaded, its collapsed state hid that panel, producing the brief flash. The fallback now derives its rendered placements from the same hidden, rightCollapsed, and bottomOpen state as the resolved surface.

AI use

Select exactly one:

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

Tool(s) and scope: Codex investigated the render path, implemented the fix and regression test, and ran the verification described above.

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

@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

Visual evidence

Same viewport, first-send state, and fixture backend.

Before

The collapsed Workbar Suspense fallback temporarily occupies the right side.

Before: right Workbar loading panel flashes during first send

After

The collapsed Workbar remains absent while the lazy surface resolves.

After: main chat remains full width during first send

@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

CI note: the current failure occurs before the desktop build and is inherited from main: packages/runtime-host/src/__tests__/execution-host-queue.test.ts still calls the removed queryTurn, stopTurn, and startTurn helpers. The focused fix is already open as #3792. This Workbar PR does not duplicate that unrelated runtime-host change; CI should be rerun after #3792 lands.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I reviewed this head and found a blocking issue.

[P1] Claimed queue-test migration not present — build fails

execution-host-queue.test.ts:248/249/271 still references queryTurn/stopTurn/startTurn missing on RuntimeHostConnection. Hosted test is red.

简体中文存在测试迁移缺失阻断。

@hqhq1025
hqhq1025force-pushed the codex/fix-workbar-first-send-flash branch from 9114804 to 5f38f4eCompareAugust 25, 2026 13:15
@hqhq1025

Copy link
Copy Markdown
ContributorAuthor

Rebased onto current main after #3796 merged. This removes the inherited Runtime Host compile failure noted in the earlier review; no unrelated queue-test migration is needed in this Workbar PR.

Local verification on 5f38f4e4c:

  • clean npm ci
  • full Desktop workspace dependency build
  • full Desktop build
  • Electron regression: a collapsed workbar never flashes during the first send (1 passed)
  • git diff --check origin/main...HEAD

Please re-review the refreshed head.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Update on 5f38f4e:

The prior P1 (queue-test still references removed RuntimeHostConnection methods) is now closed. The new head has rebased onto main with #3796's fix, execution-host-queue.test.ts no longer references queryTurn/startTurn/stopTurn (grep 0), and hosted test is now pass (run 32852297860).

The workbar-host fallback logic remains correct (no P0-P3) and is now merge-ready pending human decision.

简体中文该头 P1 已随 rebase 闭合。

@M4n5ter
M4n5terforce-pushed the codex/fix-workbar-first-send-flash branch from 5f38f4e to 4fc642aCompareAugust 26, 2026 09:07
Make the lazy Workbar fallback mirror the resolved surface visibility so creating a session cannot briefly open a collapsed panel. Add Electron coverage that records transient right-panel visibility during the first send.
Generated-by: Codex
@M4n5ter
M4n5terforce-pushed the codex/fix-workbar-first-send-flash branch from 4fc642a to 5b9627dCompareAugust 26, 2026 09:53

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The flash is real and the fix is at the right place: the lazy fallback was rendering a right-placement card unconditionally, so a collapsed workbar still flashed a loading panel while WorkbarSurface resolved on the first send.

I checked that the fix does not trade one flash for another: .maka-workbar-workspace-contents is display: contents, the cards position by grid-area, and [data-collapsed] is display: none — so omitting a card and rendering it collapsed are equivalent for layout, and nothing shifts when the surface resolves. The new e2e was verified red on main, which is the part that matters most for a one-frame regression.

Approving. One P3 and one note inline, neither blocking.

AI use: Claude Code assisted with source investigation; the analysis and conclusions are my own.

简体中文

闪烁是真实的,修复位置也对:懒加载的 fallback 无条件渲染了一张 right placement 的卡片,所以首次发送时即使工作栏是收起的,也会闪出 loading 面板。

我确认了这个修复没有用一种闪烁换另一种:.maka-workbar-workspace-contentsdisplay: contents,卡片靠 grid-area 定位,[data-collapsed]display: none——所以「不渲染」和「渲染后折叠」在布局上等价,surface 解析完成时不会发生位移。新增的 e2e 在 main 上验证为红,对一帧级别的回归来说这是最关键的一点。

Approve。行内一条 P3、一条说明,都不阻塞。

bottomOpen: boolean;
}) {
const copy = getShellCopy(useUiLocale()).app;
if (props.hidden || (props.rightCollapsed && !props.bottomOpen)) return null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P3] This re-derives a rule that already has an owner. workbar-surface.tsx:742 computes the same fact as

constvisible=!props.hidden&&(placement==='right' ? !props.rightCollapsed : props.bottomOpen);

and the fallback now expresses it a second time, in a different shape. The two agree today, which is why this is P3 — but a fallback drifting from the resolved surface is exactly the bug this PR is fixing, so leaving a second copy of the rule behind reopens the same seam.

The surface also renders both cards always and hides the invisible one with data-collapsed, while the fallback omits it instead. Same pixels (the CSS makes them equivalent), different DOM. Exporting one predicate and letting the fallback mirror the surface's structure collapses both differences, and the hidden || (rightCollapsed && !bottomOpen) early return then falls out — it is already implied by the placement list being empty.

watch.visibleRightWorkbar = true;
}
};
const observer = new MutationObserver(inspect);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The observer detects by sampling rather than by record: inspect() re-queries the live DOM, so a card added and removed inside one mutation batch leaves records behind but nothing for the query to find. It caught the real regression — you verified it red on main — so this is not a problem today, just the part that would quietly stop catching things. Reading the added nodes out of records would make the detection independent of how fast the flash is.

@Astro-Han
Astro-Han merged commit 0a3390c into apache:mainAug 26, 2026
2 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.

2 participants

@hqhq1025@Astro-Han