fix: #172 follow-ups after #169 (analytics warm-up retry, drawer close contrast, IATA oldest test, RX fit race) - #175
Conversation
#172) Part #172 of test-1659-analytics-warmup.js runs the real app.js api() and the real analytics page in one vm with a fake fetch and a fake clock. The server answers 503 + Retry-After: 5 on rf/topology/channels until its background load is done (up to the 60 s force-open). - a 45 s warm-up must end with data, with a "still loading" state and no error on the way; - a permanent 503 must keep retrying for at least 90 s and give up by about 120 s; - destroy() during the warm-up must stop the retries and write nothing; - a new load during the warm-up must leave one retry timer and render the data once. Red on master: the error shows at 31 s (api() stops after 6 attempts), retries keep running after destroy(), and three retry timers are pending. Relates to #172 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…172) Since #145 the server answers 503 + Retry-After on rf/topology/channels until its whole background load is done, for up to its 60 s force-open. api() gave up after 6 attempts (about 30 s at Retry-After: 5) and the page showed "Failed to load". - api() takes retry503:false for a caller that retries on its own. Its errors now carry the HTTP status, and a 503's valid Retry-After as retryAfterSeconds. Other callers keep the old 6-attempt loop. - loadAnalytics() retries a 503 on the server's interval while the next attempt starts within 120 s of the load's start, and shows a "still loading" status meanwhile; only then does it show the error. - Like the distance tab (#120), each load and destroy() bump _loadGen and clear the one retry timer, so a superseded load or a left page never renders. The retry delay helper is shared with the distance tab. Also adds a test that a slow response of a superseded load does not render over the newer one (red on master; it was added here because a mutant without the success-path generation check survived the first four). Relates to #172 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…172) Step (k) of test-nav-drawer-1064-e2e.js now walks every customizer preset (read from the preset buttons) in the light and the dark theme, checks that the preset's --nav-bg2 is applied, waits for the colour transitions to settle, and requires 4.5:1 on the header background for both the title and the close button (the close button used to need 3:1). Red on master: the close button's --nav-text-muted measures 3.38:1 (forest/light), 4.05:1 (forest/dark), 3.03:1 (sunset/dark) and 3.67:1 (mono/dark). Relates to #172 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#169 put the drawer header on --nav-bg2. The close button kept --nav-text-muted, which is under 4.5:1 on --nav-bg2 in four presets: forest/light 3.38, forest/dark 4.05, sunset/dark 3.03, mono/dark 3.67. The close button now uses --nav-text, the colour of the title beside it, which passes on the same background in every preset and theme. This is the least invasive fix: one declaration on one control. Changing the four presets' --nav-text-muted instead would also recolour every inactive navbar link and drawer item, and would not protect an operator's own theme. Hover and focus stay visible through the hover background and the focus outline. Relates to #172 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TestIATADropThrottleNoResweepAfterPartialSweep fills the throttle table with entries at +0 and +30m, lets a new region at +1h sweep out the +0 half, refills the table, and then sends 100 new regions between +1h and +1h29m. None of them may sweep again: the oldest kept entry (+30m) cannot expire before +1h30m, where exactly one more sweep must run. Green on master, which is correct. Without `t.oldest = oldest` in shouldWarn it fails with 101 sweeps instead of 1; every other IATA test stays green with that line removed. Test only: no production change. Relates to #172 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Tests 14a-c in test-issue-124-rx-coverage-viewport.js open an observer-only link, so fitToObserver() requests the observer's extent for days=7, and keep that response pending. They then switch days to 30 (14a), pick All (14b) or pick another observer (14c), release the stale response, and require that fitBounds is not called for it. 14c also requires that the newer observer's extent still fits the map. Red on master: in all three the stale extent calls fitBounds. Relates to #172 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fitToObserver() fetches the observer's extent and then calls fitBounds. Only isLive(gen) guarded the response, so switching days, picking All or picking another observer while it was in flight still moved the map to the old request's extent. Same class of bug as item 3 of #150. A fit request now takes a sequence number like coverage and leaderboard (#169) and applies only while it is the latest fit (isLatestFit). A newer fit, a days switch and All bump the sequence, so the stale response (and its error path) does nothing. Relates to #172 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…clicks during warm-up (#172) - api() shares one in-flight promise per path, so a retry503:false caller and a default caller get each other's behaviour (3 tests). - Clicking a data tab while the analytics are still loading throws a TypeError instead of showing the loading state, and the warm-up retries overwrite a tab that fetches its own data (7 tests). - Pin what was untested: a 500/404 fails at once without retries, the retry follows Retry-After clamped to 1..30 s, and the loading state is role="status" (6 tests, green on the current code). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vwXk6z7Bcv1AmN2qd6WSZ
The tab-click tests ran the tabs on the overview's minimal stub, so a correct guard still failed inside renderRF/renderTopology/... on data those tabs never get from the server. They now use the CI fixture server's real responses for the five shared endpoints (arrays cut to two items, node names and keys anonymised) and check that the clicked tab, not the overview, renders after the warm-up. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vwXk6z7Bcv1AmN2qd6WSZ
…me retry503 (#172) _inflight was keyed on the path alone. A default caller that joined a retry503:false request got its 503 after one attempt, and a retry503:false caller that joined a retrying request waited out the whole retry loop (up to ~31 s) without the cancellation analytics.js relies on. The key now includes the flag; default callers keep the bare path as their key. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vwXk6z7Bcv1AmN2qd6WSZ
…ding (#172) While the shared load had no data, a click on Overview, RF, Topology, Channels, Hash Stats or Hash Issues rendered from an empty _analyticsData and threw an uncaught TypeError (snrValues, ...). The window is now up to 120 s instead of ~30 s. renderTab() now shows the load's current status (loading, still loading or the error) on those six tabs until the data is there, and the load renders the selected tab when it is. The load writes its status only while one of those tabs is shown, so its 503 retries no longer overwrite a tab that fetches its own data. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vwXk6z7Bcv1AmN2qd6WSZ
|
Review feedback addressed (commits
Not changed: the drawer close-button hover colour. Measured on the hover background in 8 presets × light/dark, only Mutants (each in a scratch copy)
Runs
Browser (headless Chromium, local server, CI fixture; 503s mocked with
|
…e synthetic (#172) The fixture still had two real observer pubkeys from e2e-fixture.db as object keys in topoData.perObserverReach. They are now synthetic 64-hex keys like the other ids. The values, the key order and the shape are unchanged. A scan of every key and value against nodes, observers, transmissions (hash, from_pubkey, decoded_json) and neighbor_edges finds no real ids or names left. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TdKwzUPntKQHjQEKpMXefc
…he status, theme-refresh during a load (#172) Re-review gaps in test-1659-analytics-warmup.js (34 tests, was 28): - The self-fetching-tab test wrote its own marker after the click, so it passed even when renderTab skipped the tab's render function (guard without LOAD_TABS.has(tab)). It now runs for Route Patterns, Roles and Nodes and requires that tab's own fetch and its own output during the warm-up, and that the 503 retries leave the output alone. - After a failed load and a region change, a data tab clicked during the new load must show "Loading analytics", not the old "Failed to load". - theme-refresh on Overview and RF during the warm-up must not throw and must keep the loading state; the data renders after the warm-up. The page's window listener is captured and called like app.js does. - A slow 500 of a superseded load must not be shown during the newer load (the gen check in the error path had no test). Test only. On 1c510d4 the theme-refresh tests fail with the reported TypeError (totalTransmissions, snrValues). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TdKwzUPntKQHjQEKpMXefc
|
Review feedback addressed (commits Test and fixture only.
Mutants (each in a scratch copy,
|
| Mutant | Caught by |
|---|---|
M8: guard without LOAD_TABS.has(tab) |
"clicking subpaths / roles / nodes during the warm-up renders that tab…" (3) |
M6: no _loadStatusHtml = LOADING_HTML on a new load |
"after a failed load, a data tab clicked during the next load shows Loading…" |
| renderTab guard removed | the 6 data-tab click tests, the 2 theme-refresh tests, the M6 test |
| guard moved to the click handler (theme-refresh only unguarded) | the 2 theme-refresh tests |
_inflight keyed on the path alone |
the 3 in-flight tests |
no e.status === 503 check |
"a 500 / a 404 shows the error at once", the M6 test |
load ignores Retry-After (fixed 5 s) |
"Retry-After: 1 / 12 / 60" |
no err.retryAfterSeconds in app.js |
"Retry-After: 1 / 12 / 60" |
| load status written over any tab | the 3 self-fetching-tab tests |
no role="status" |
"Retry-After: …" (4), 6 tab-click tests, 2 theme-refresh tests |
| no gen check after the responses | "a slow response of a superseded load…" |
| no gen check in the error path | "a slow error of a superseded load…" (new) |
| warm-up cap 30 s | 12 tests, including "a warm-up longer than 30 s…" |
destroy() keeps the retry timer |
"leaving the page during the warm-up…" |
All 14 mutants are caught on the branch and on a local merge with origin/master. Against the old test file, M8, M6, "guard in the click handler" and "no gen check in the error path" survive.
Runs
test-1659-analytics-warmup.js: 34/34 (was 28).- On the branch,
test-app-api-inflight-cleanup-rejection.js8/8,test-warmup-banner.js13/13 andtest-issue-1375-scope-stats-fetch.js9/9 pass.test-frontend-helpers.jsfails 2 (favStar), andtest-analytics-channels-integration.jsfails 1, both already fixed on master. - On a local merge with
origin/master(cbd562cd): all of the above are green, includingtest-frontend-helpers.js707/707 andtest-analytics-channels-integration.js24/24. - Local full validation at
7ad6defd(Go with-race, the JS unit tests in the CI list, Playwright E2E on the CI fixture): 204 PASS, 0 FAIL. - Fork guards: 9
github.repository == 'Kpa-clawbot/CoreScope', unchanged.
🤖 Generated with Claude Code
Review — CS-Cloud re-review PR#175 runde 3 — head 7ad6defDom: APPROVE with nits Delta review of Checks
Test results [T]
Mutants [T]Each mutant was applied to a scratch copy of the head and run against both the round-2 test and the round-3 test. "Survived" means the test file stayed green.
So M8a, M6, T3 and T5 survived round 2 and are now killed. T1 and T2 were already killed by the tab-click tests, because they share the Nits
Not checked
Generated by Claude Code |
Summary
Four follow-ups after #169, as listed in #172. Each item has its own test commit and fix commit, except item 3, which is test only.
Relates to #172
Relates to #150
1. Analytics outlasts the server warm-up (item 5 of #150)
Since #145 the server answers
503+Retry-After: 5on/api/analytics/{rf,topology,channels}until its background load is done, for up to the 60 s force-open (cmd/server/analytics_warmup_1659.go).api()gave up after 6 attempts (about 30 s), so the page showed "Failed to load".api()takesretry503: falsefor a caller that retries on its own. Its errors now carry the HTTPstatus, and a 503's validRetry-AfterasretryAfterSeconds. Other callers keep the old 6-attempt loop.loadAnalytics()retries a 503 on the server's interval while the next attempt starts within 120 s of the load's start, and shows a "still loading" status (role="status") meanwhile. Only after that does it show the error.destroy()bump_loadGenand clear the single retry timer. A superseded load or a page the user has left never renders._retryAfterDelayMs).The 120 s cap is a constant. Per AGENTS.md rule 8, exposing it in the customizer is left for later.
Perf: this is not a hot path. During a warm-up, each open analytics page sends at most three requests every 5 s, for at most 120 s. Successful endpoints are cached by
api()and are not fetched again.2. Drawer close-button contrast in every preset
The close button used
--nav-text-mutedon--nav-bg2. That falls below 4.5:1 in forest/light (3.38), forest/dark (4.05), sunset/dark (3.03) and mono/dark (3.67).The fix gives the button
--nav-text, the same colour as the title next to it, which passes on that background in every preset and theme. It is one declaration on one control, so it is the least invasive option. Changing--nav-text-mutedin the four presets would also recolour every inactive navbar link and drawer item, and it would not help an operator's own theme. Hover and focus are still visible through the hover background and the focus outline.3. IATA throttle:
oldestrefresh after a partial sweep (test only)TestIATADropThrottleNoResweepAfterPartialSweepdoes a partial sweep, refills the table, and sends 100 new regions before the next deadline. Exactly one sweep may happen in that window, and exactly one more at the deadline. No production change was needed.4. RX Coverage: stale
fitToObserverresponseA fit request now gets a sequence number, like the coverage and leaderboard requests in #169, and it only applies while it is the latest fit (
isLatestFit). A newer fit, a change of days and the All button each bump the sequence, so a stale response, or its error path, does nothing.Tests
test-1659-analytics-warmup.js(#172 part, 5 tests)test-nav-drawer-1064-e2e.jsstep (k), 8 presets × light/darkTestIATADropThrottleNoResweepAfterPartialSweep-racetest-issue-124-rx-coverage-viewport.js14a–cMutants (each run in a scratch copy) and the test that catches each one:
destroy()no longer cancels → the leaving-the-page test;api()retry loop kept → three tests;--text-muted→ step (k), 10 combinations red;t.oldest = oldestremoved → the new IATA test (101 sweeps instead of 1);isLive→ 14a–c;setDaysdoes not drop the fit → 14a;The following runs were identical on master and branch:
test-frontend-helpers.js: 705 passed, plus the 2 known favStar failures.test-analytics-channels-integration.js: the same single failure (channels.js sidebar link).test-e2e-playwright.jsagainst a local fixture server: stops fail-fast at "Version info lives on Perf dashboard".scripts/check-xss-sinks.sh --diff origin/masterandscripts/check-css-vars.jsare clean. The fork guards indeploy.ymlare unchanged: 9.Browser validation (local server, scratch proxies)
Retry-After: 5for 45 s. On desktop and mobile the page showed "still loading", then the data at about 45.9 s, and never an error. Master showed "Failed to load" at about 30.8 s.🤖 Generated with Claude Code