|
| 1 | +# run_smoke.py: three runner variants across 10 repos, no sync mechanism |
| 2 | + |
| 3 | +Type: maintenance |
| 4 | +Target: ci |
| 5 | +Repos: |
| 6 | +- @PyAutoHands |
| 7 | +Difficulty: medium |
| 8 | +Autonomy: supervised |
| 9 | +Priority: normal |
| 10 | +Status: draft |
| 11 | + |
| 12 | +> **Re-scoped 2026-08-05.** The original finding below is preserved for history |
| 13 | +> but its two headline claims are now measured to be wrong: step 1 is already |
| 14 | +> done, and "9 copies in 5 revisions" conflates three structurally different |
| 15 | +> programs. Read the correction block first — it is the current statement of |
| 16 | +> the task. |
| 17 | +
|
| 18 | +## Original finding (2026-07-25, during the notebook skip-guard fix) |
| 19 | + |
| 20 | +Every workspace's PR smoke gate runs its own copy of |
| 21 | +`.github/scripts/run_smoke.py`. There are **9 copies across the workspace |
| 22 | +repos in 5 distinct revisions** — they have already drifted: |
| 23 | +autofit_workspace + autolens_workspace share one revision; HowToLens + |
| 24 | +HowToGalaxy + autofit_workspace_test share another; autogalaxy_workspace, |
| 25 | +autogalaxy_workspace_test, autolens_workspace_test and autocti_workspace_test |
| 26 | +are each unique. PyAutoHeart's reusable smoke-tests.yml deliberately leaves |
| 27 | +the runner in the workspace, but nothing keeps the copies aligned. |
| 28 | + |
| 29 | +Immediate consequence: PyAutoHands#198 taught the authoritative executor |
| 30 | +(`autohands/build_util.py::execute_notebook`) to treat a clean `SystemExit: 0` |
| 31 | +notebook exit as a PASS (the optional-dep skip-guard idiom), but the 9 smoke |
| 32 | +copies still carry their own `execute_notebook` and keep reporting the |
| 33 | +spurious FAIL until each adopts it. Adoption is ~2 lines per repo (they |
| 34 | +already import `env_config` and `build_util.py_to_notebook` from PyAutoHands); |
| 35 | +the exact snippet + full copy inventory is documented in |
| 36 | +PyAutoHands `docs/internals.md`. |
| 37 | + |
| 38 | +## Correction (2026-08-05, measured during the jupyter-guard fix) |
| 39 | + |
| 40 | +Measured across the checkouts, not inferred. There are **10** copies, not 9, |
| 41 | +and they are **three different programs**, not five revisions of one: |
| 42 | + |
| 43 | +| Variant | Repos | Lines | Notebook leg | `is_clean_skip_exit` | |
| 44 | +|---|---|---|---|---| |
| 45 | +| **workspace** | autofit_workspace, autogalaxy_workspace, autolens_workspace | ~266 | yes | **already adopted** | |
| 46 | +| **workspace_test** | autofit_workspace_test, autogalaxy_workspace_test, autocti_workspace_test | 113 | no | n/a | |
| 47 | +| **workspace_test + timeout** | autolens_workspace_test | 193 | no | n/a | |
| 48 | +| **HowTo** | HowToLens, HowToGalaxy, HowToFit | 75 | no | n/a | |
| 49 | + |
| 50 | +Consequences for the original task, in order of how much they change it: |
| 51 | + |
| 52 | +1. **Step 1 is done, and was never applicable beyond three repos.** The |
| 53 | + skip-guard is adopted in all three notebook-capable copies. The other seven |
| 54 | + have no `execute_notebook` at all — they never shell out to `jupyter`, so |
| 55 | + there is no spurious FAIL for them to fix. The acceptance criterion "a |
| 56 | + notebook exiting via the skip-guard passes every workspace's PR smoke gate" |
| 57 | + is already met, because only three gates run notebooks. |
| 58 | +2. **The variants differ by feature, not only by drift.** `workspace_test` |
| 59 | + exposes `load_smoke_scripts`/`run_one` and no notebook machinery; |
| 60 | + `autolens_workspace_test` additionally carries `TIMEOUT_SECS` |
| 61 | + (`BUILD_SCRIPT_TIMEOUT`) and a `_kill_group` process-group kill that no |
| 62 | + other copy has. That is a capability, not staleness — a naive |
| 63 | + "make them byte-identical" would delete it. |
| 64 | +3. **The HowTo tier is already the proposed end-state.** Those three are |
| 65 | + 75-line delegators (`PROJECT = "howtolens"`, straight into PyAutoHands |
| 66 | + `build_util`) — the thin-wrapper design step 2 asks whether to build |
| 67 | + already exists in-tree as a working precedent to copy. |
| 68 | +4. Real remaining drift inside the workspace variant is now **two lines**: |
| 69 | + autofit_workspace and autolens_workspace are byte-identical; autogalaxy's |
| 70 | + only divergence is an unused `_BUILD_DIR` intermediate variable. The |
| 71 | + jupyter-guard fix (autolens_workspace#470) landed the identical patch in all |
| 72 | + three, so it did not widen this. |
| 73 | + |
| 74 | +## Task (re-scoped) |
| 75 | + |
| 76 | +1. ~~Roll the 2-line skip-guard adoption across the copies.~~ **Done** — verify |
| 77 | + and close out, do not redo. |
| 78 | +2. Decide the shared-module question **per variant**, not globally, using the |
| 79 | + HowTo delegator as the reference shape: |
| 80 | + - Is one PyAutoHands-owned runner with per-repo config the right target, or |
| 81 | + two (notebook-capable and script-only)? |
| 82 | + - Does `autolens_workspace_test`'s timeout/kill behaviour get promoted to |
| 83 | + everyone, or does that repo keep a documented divergence? **Answer this |
| 84 | + before writing any code** — it is the only place consolidation destroys |
| 85 | + behaviour. |
| 86 | +3. Implement whichever shape is chosen, one PR per repo. |
| 87 | +4. Drop the vestigial `_BUILD_DIR` line in autogalaxy_workspace if the |
| 88 | + workspace variant is not being replaced wholesale. |
| 89 | + |
| 90 | +## Acceptance |
| 91 | + |
| 92 | +- A stated, written decision on the timeout/kill divergence — promoted or |
| 93 | + documented-as-intentional — before any repo is touched. |
| 94 | +- Each variant is either a thin wrapper over a PyAutoHands-owned module, or |
| 95 | + carries a documented reason why it diverges. |
| 96 | +- No repo loses behaviour it has today; `autolens_workspace_test` still |
| 97 | + enforces its per-script timeout. |
0 commit comments