Harden CI + finish #33: import guard (catches a 4th split regression) + e2e condition wait#2
Merged
Conversation
trace_mask_to_svg and trace_color_to_svg call log_subprocess_lines(log, lines) right after running the vtracer subprocess, but engines.py only imported run_subprocess from hvserver.jobs — so those two trace paths NameError'd the moment the subprocess returned. Caught by the new import guard (tests/test_imports.py), not by e2e (its vectorize path doesn't reach these two functions). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds tests/test_imports.py: a static check that flags any bare Name(Load) used but bound nowhere in its module — the missing-import NameError class the god-file splits (#25/#28/#29/#30) kept introducing (pipeline.py's pixelvec_config / resolve_capability_step, docks.js's showContextMenu, engines.py's log_subprocess_lines), all of which shipped because ast.parse / node --check can't see them and the early e2e stall hid the runtime paths. Uses each module's real runtime namespace (so `import *` resolves), unions in every function/lambda local + except/comprehension binding, and skips annotation-only names and dunders, so attribute access and forward-ref annotations don't false-positive. ~1s, no optional ML deps. Wired into ci.yml (+ the capability suite) and release.yml. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds a settle(page, js) helper that waits for the actual end-state instead of outlasting a debounce with a fixed sleep — fast when ready, bounded otherwise, and a swallowed timeout means the following assertion still reports the real state with context instead of a raw Playwright abort. Applied to the colour recents-debounce (820ms -> condition on the swatch landing in localStorage). Opportunistic per #33: the remaining sleeps are sub-120ms post-pointer settles or the colour-undo coalesce debounce, which has no self-verifying next-line condition (closing the panel early would drop the undo entry) — left as-is. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to the audit-remediation merge — closes the last open task (#33) and hardens CI.
CI guard (the substantive part)
The two serious pipeline regressions in the last PR were missing imports after the god-file splits — invisible to `ast.parse`/`node --check` and only caught because the e2e finally ran the paths. This adds `tests/test_imports.py`, a ~1s tool-free static check that flags any name used but bound nowhere in its module. On its first run it caught a 4th, still-latent regression: `hvserver/engines.py` calls `log_subprocess_lines` (in jobs.py) without importing it, so `trace_mask_to_svg`/`trace_color_to_svg` would NameError right after the vtracer subprocess. Fixed (commit 1), and wired the guard into `ci.yml` + `release.yml` (also added the capability suite to CI).
Note: the full Playwright e2e is not added to CI on purpose — it asserts real spandrel/onnxruntime/opencv availability and runs a real denoise, needing the heavy ML toolchain the app keeps out of `requirements.txt`. The static guard covers the regression class that actually bit, cheaply.
#33 (the open task)
Adds a `settle(page, js)` helper and converts the worst fixed sleep (the 820ms colour-recents debounce) to a condition wait. Opportunistic per the task: the rest are sub-120ms post-pointer settles or the colour-undo coalesce debounce (no self-verifying condition; converting risks dropping undo) — left as-is with rationale.
Verified: e2e 371/371, test_smoke + test_capabilities + test_imports green locally.