TST make the flat-keyframe render-size test robust on slow machines

[kroq-gar78]

test_changing_the_render_size_reframes_flat_keyframes failed often but inconsistenly on CI with TypeError: 'NoneType' object is not subscriptable (e.g. on this failed run).

WebApp.send gives up after 2 s and returns None. The test previously read the keyframes right after starting a render. On a slow machine (like our CI) the page was still busy drawing the frame and didn't answer in time.

Changes:

• Read the keyframes after the render finishes. The render updates them before it starts drawing, so the test still checks the same thing.
• Add a _js_query helper that retries a read until the page answers, for up to 60 s.

With these changes, the test reliably passes with fewer cores.

[jwparks]

@claude review this, related PR #752 #745

[claude]

Claude finished @jwparks's task in 22s —— View job

---

Review of PR #764: make the flat-keyframe render-size test robust on slow machines

• Read the diff and the surrounding code
• Checked WebApp.send / _get_response in cortex/webgl/serve.py, which NF webgl views animation gui #752 and NF optimize 4D timeseries performance and add interactive timeseries panel #745 touch
• Posted this review

Verdict: looks good. I have a few non-blocking suggestions.

What I checked

• The retry works. On timeout, WebApp.send returns [None] * n_clients (serve.py:413-416). _js_run's len(resp) == 1 assert still passes, so _js_query gets None and retries. The failure was a TypeError from subscripting that None, and this fixes it.
• Stale replies can't cause a mismatch. The request-id tagging in send / _get_response discards late replies. A retry therefore can't read the answer to an earlier timed-out request, even though the page is slow.
• Moving the read after wait_for_download is sound. The PR description says the render updates the keyframes before it starts drawing. The assertion therefore still checks the reframe, and the page is idle by the time it reads.
• The annotations are fine. JSON and JSProxy are both exported from cortex.webgl.serve, and list[Any] matches the Python versions CI covers. That matches the repo's "annotate new code" guidance.

Suggestions (non-blocking)

1. None is a legitimate JS return value. _js_query treats None as "unanswered". A read that really returns null/undefined would spin for 60 s and then fail with the misleading message "went unanswered". That's fine for the two current callers, which both return objects. Say in the docstring that it is only for reads that return non-null values.
2. Tight loop. Each retry already blocks for up to 2 s inside send, so a busy page isn't hammered. A short time.sleep(0.1) before retrying would still help on a fast-failing path, such as a disconnected socket, where send returns None immediately. The queue.get timeout can also be 0 there. Without the sleep, that case spins at full CPU until the deadline.
3. Other reads right after a render can still flake. _js_run(handle, "window.viewer._animPanel.render", []) and the earlier setRenderSize / addKeyframe calls still use _js_run. _js_run asserts len(resp) == 1 but never checks for None, so a timed-out call is silently ignored. For render that probably doesn't matter, since the download wait follows. If more flakiness shows up, those setter calls are the next place to look.
4. Style nit. assert cond, \ followed by a continuation line (_js_query) would read better as assert (cond), (msg) or with a plain if ...: pytest.fail(...). The rest of the file doesn't use backslash continuations.

No functional issues found. I did not run the test suite, because it needs Playwright Chromium and a slow-machine reproduction.
· branch fix-keyframe-test-failure

[jwparks]

I tested this commit locally. To mimic a slow CI runner, I pinned the test to fewer CPU cores:

Setup	Before (main)	This PR
1 CPU core	5 of 5 failed	5 of 5 passed
2 CPU cores	not tested	3 of 3 passed
All cores	passed	3 of 3 passed

Before this PR, the test failed every time on one core with the same TypeError we saw in CI. Overall, looks good to me!

[kroq-gar78]

From Claude's review:

Suggestions (non-blocking)

1. None is a legitimate JS return value. `

I think this is clear from the implementation of _js_query.

2. Tight loop. Each retry already blocks for up to 2 s inside send, so a busy page isn't hammered. A short time.sleep(0.1) before retrying would still help on a fast-failing path [...]

This is true, but I think the 2-sec. sleep was chosen because of synchronization issues. I will address this in a later PR that can hopefully tighten all of the sleeps.

3. Other reads right after a render can still flake. `[...] If more flakiness shows up, those setter calls are the next place to look.

See above -- hopefully this will be less of an issue in the future.

4. Style nit. assert cond, \ followed by a continuation line (_js_query) would read better as assert (cond), (msg) or with a plain if ...: pytest.fail(...). The rest of the file doesn't use backslash continuations.

We don't have strict style guidelines, so I'm going to ignore this.