mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-09-13 04:46:13 +00:00
Closes #1925. This is the flake that failed #1942 and #1871, neither of which touches the feature. It is not a test problem. ## Cause Every page load fires exactly one delayed, full re-render of the active analytics tab: 1. `app.js` starts `/api/config/theme` without gating navigation on it, deliberately. 2. When it resolves, `_customizerV2.init()` runs `applyCSS()`, which dispatches `theme-changed`. 3. `app.js:1188` debounces that by 300 ms and dispatches `theme-refresh`. 4. `analytics.js:230` answered it with `renderTab(_currentTab)`. For the neighbor-graph tab step 4 is destructive. `renderTab` replaces `el.innerHTML`, so the role checkboxes are recreated with their defaults and companion is silently re-checked, and `_ngState` is rebuilt from the full 1400-node graph. The count is back over the 1000 limit, so `#ngSkipMsg` returns and the canvas is hidden. The re-entrancy epoch guard cannot prevent it. That guard stops a superseded `tick()` loop; this is a legitimate new top-level render pass that resets the very inputs the guard protects downstream of. **One mechanism produces both documented failure modes**, decided only by where that single re-render lands: | lands | result | |---|---| | before the first uncheck | harmless, test passes | | between an uncheck and the next `waitForFunction` poll | mode 1, the 15 s timeout | | after `waitForFunction` succeeded, before the follow-up `evaluate` | mode 2, "expected #ngSkipMsg gone again" (#1942) | Measured on an idle machine: the test's final evaluate at 542 ms, `theme-refresh` at 636 ms, `#ngSkipMsg` re-added at 667 ms. It passes locally by about 90 ms. On a loaded runner the test's Playwright round trips stretch while `theme-refresh` still lands at theme-fetch latency plus 300 ms, so it arrives mid-test. ## Fix On `theme-refresh`, restart the renderer instead of rebuilding the tab when the neighbor-graph tab is active and has state. Four lines. This stays theme-correct: node colors are read live per frame from `window.ROLE_COLORS`, role swatches use `.role-swatch--{role}` CSS tokens, stats and the skip message use CSS variables, and the one cached theme value, `_labelColor = cssVar('--text-primary')`, is re-read on restart at `analytics.js:3375`, inside `startGraphRenderer`. When `_ngState` is null it falls through to the old path. ## Verification Measured, not asserted: - **Deterministic reproduction** (hold `/api/config/theme` until just before the second filter-down, then stall 500 ms before the final evaluate; no synthetic events dispatched): **2 of 2 fail** on unmodified master with the exact #1942 message, **3 of 3 pass** with this fix. - The **unmodified** E2E test passes against the fixed build. - With the tab open and a filter applied, a `theme-refresh` leaves the filter intact, the canvas present, and produces no page errors. - The **regression test added here fails on unmodified master** with `theme-refresh reset the role filter (companion re-checked)` and passes with the fix. It dispatches the event directly, so it tests the cause instead of waiting for the race to appear. ## What this does not cover The same startup re-render silently discards user interaction in the first second or so on **any** analytics tab, not just this one. A user who clicks quickly after load loses that click. This change covers the neighbor-graph tab, because that is what #1925 is about and what is failing CI. The general fix, for example skipping the startup refresh when the effective config changed nothing, deserves its own issue rather than being smuggled in here. Side observation while tracing: `theme-changed` fires twice at startup, at about 311 ms and 323 ms. The debounce collapses them, so it is harmless, but it means the customizer pipeline runs twice. I did not identify the second dispatcher. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+14
-1
@@ -228,7 +228,20 @@
|
||||
}
|
||||
|
||||
// Re-render when distance unit or theme changes
|
||||
_themeRefreshHandler = function () { renderTab(_currentTab); };
|
||||
_themeRefreshHandler = function () {
|
||||
// #1925: never full-rebuild the neighbor-graph tab on theme-refresh.
|
||||
// Every page load fires one theme-refresh ~300ms after
|
||||
// /api/config/theme resolves (app.js dispatches 'theme-changed', then
|
||||
// debounces 300ms). renderTab() here replaced el.innerHTML, which reset
|
||||
// the role checkboxes to their defaults and rebuilt _ngState from the
|
||||
// full graph, discarding any filtering the user had applied in the
|
||||
// meantime. Restarting the renderer keeps the current filter state and
|
||||
// still picks up the new theme: node colors are read live per frame
|
||||
// from window.ROLE_COLORS, role swatches use CSS tokens, and the one
|
||||
// cached value (_labelColor) is re-read on restart.
|
||||
if (_currentTab === 'neighbor-graph' && _ngState) { startGraphRenderer(); return; }
|
||||
renderTab(_currentTab);
|
||||
};
|
||||
window.addEventListener('theme-refresh', _themeRefreshHandler);
|
||||
|
||||
loadAnalytics();
|
||||
|
||||
@@ -204,6 +204,34 @@ function buildFixture() {
|
||||
if (reFiltered.canvasDisplay === 'none') fail('second filter-down: expected #ngCanvas visible again');
|
||||
console.log(' ✓ second filter-down: render restored across repeated cycles');
|
||||
|
||||
// --- #1925: a theme-refresh must not discard the applied filter.
|
||||
// Every page load fires one 'theme-refresh' about 300ms after
|
||||
// /api/config/theme resolves. It used to call renderTab(), which replaced
|
||||
// el.innerHTML (resetting the role checkboxes to their defaults) and
|
||||
// rebuilt _ngState from the full graph, putting #ngSkipMsg back. On an
|
||||
// idle machine the test finished ~90ms before that landed; under CI load
|
||||
// it landed mid-test, which is both failure modes of #1925.
|
||||
// Dispatching the event directly makes that deterministic, so this asserts
|
||||
// the cause rather than waiting for the race to show up.
|
||||
await page.evaluate(() => window.dispatchEvent(new CustomEvent('theme-refresh')));
|
||||
await page.waitForTimeout(500);
|
||||
const afterTheme = await page.evaluate(() => {
|
||||
const canvas = document.getElementById('ngCanvas');
|
||||
const cb = document.querySelector('#ngRoleChecks input[data-role="companion"]');
|
||||
return {
|
||||
hasSkip: !!document.getElementById('ngSkipMsg'),
|
||||
canvasDisplay: canvas ? getComputedStyle(canvas).display : '(no canvas)',
|
||||
companionChecked: cb ? cb.checked : '(no checkbox)',
|
||||
};
|
||||
});
|
||||
if (afterTheme.companionChecked !== false) {
|
||||
fail('theme-refresh reset the role filter (companion re-checked): '
|
||||
+ JSON.stringify(afterTheme));
|
||||
}
|
||||
if (afterTheme.hasSkip) fail('theme-refresh brought #ngSkipMsg back: ' + JSON.stringify(afterTheme));
|
||||
if (afterTheme.canvasDisplay === 'none') fail('theme-refresh hid #ngCanvas again');
|
||||
console.log(' ✓ theme-refresh preserves the applied filter (#1925)');
|
||||
|
||||
await browser.close();
|
||||
|
||||
console.log('\nPASS: #1758 neighbor-graph filter re-render lifecycle holds');
|
||||
|
||||
Reference in New Issue
Block a user