fix(build): the lift key is taken again only if the lift's other inputs did not change

github-sync commented

2026-11-12 06:22:42 +00:00

Owner

@jordanella opened this pull request on GitHub · 2026-11-12 06:12 UTC · original
Merged on GitHub in 331efae3c3

#2674 can certify stale lifted C: if a translator file is edited while a lift runs, the key taken after the lift describes the new file, but the C came from the old one, and the next build skips a lift it needs. mooki000 found and reproduced this on #2674 just before it merged; this closes it.

Change

A new lift_key(..., planning=False): everything the key hashes except the two files the lift writes for the next one (image/chunks.txt, image/hot_pieces.txt).
translate() takes that as the lift starts and again after it, and takes the key again only if it's unchanged. If something else changed while the lift ran, the key from before the lift stays, and the next build translates again, as before #2674.
#2674's case is unchanged: a new work folder's second build still skips the lift, since only the lift's own files changed.

This is mooki000's suggested shape: refresh the self-produced inputs only after confirming the rest are unchanged.

Testing

tests/test_lift_cleanup_error.py: a new case where an input changes during the lift writes the pre-lift key, and the key taken after doesn't validate. Against main's make_game.py it fails; with this change it passes. The existing cases still see lift → key taken again → key written.
tests/test_lift_cache.py:
- The source order: the inputs snapshot is taken before nx2/lift.py runs, and the check comes before the key is taken again.
- planning=False ignores changes to hot_pieces.txt and chunks.txt, but follows the game's code and the hot list.
- Fails against main's make_game.py, passes with this change.
Every check that reads make_game.py (52), on Windows, passes apart from test_cpp_preflight, test_linux_update and test_prepare_stall, which fail the same way on a clean main here.
Not run: mooki000's gist fixture, which this machine's tool permissions wouldn't let me run. Its guarded variant is the same check this PR adds; a run with --variant pointed at this branch's commit would confirm it.

🤖 Generated with Claude Code

> [@jordanella](https://github.com/jordanella) opened this pull request on GitHub · 2026-11-12 06:12 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2686) > **Merged on GitHub** in https://git.magicbean.dev/MagicBeanDev/Hookshot/commit/331efae3c3c2c626614678487afd44681f0e758a #2674 can certify stale lifted C: if a translator file is edited while a lift runs, the key taken after the lift describes the new file, but the C came from the old one, and the next build skips a lift it needs. mooki000 found and reproduced this on #2674 just before it merged; this closes it. ## Change - **A new `lift_key(..., planning=False)`:** everything the key hashes except the two files the lift writes for the next one (`image/chunks.txt`, `image/hot_pieces.txt`). - **`translate()` takes that as the lift starts and again after it,** and takes the key again only if it's unchanged. If something else changed while the lift ran, the key from before the lift stays, and the next build translates again, as before #2674. - **#2674's case is unchanged:** a new work folder's second build still skips the lift, since only the lift's own files changed. This is mooki000's suggested shape: refresh the self-produced inputs only after confirming the rest are unchanged. ## Testing - **`tests/test_lift_cleanup_error.py`:** a new case where an input changes during the lift writes the pre-lift key, and the key taken after doesn't validate. Against `main`'s `make_game.py` it fails; with this change it passes. The existing cases still see lift → key taken again → key written. - **`tests/test_lift_cache.py`:** - The source order: the inputs snapshot is taken before `nx2/lift.py` runs, and the check comes before the key is taken again. - `planning=False` ignores changes to `hot_pieces.txt` and `chunks.txt`, but follows the game's code and the hot list. - Fails against `main`'s `make_game.py`, passes with this change. - **Every check that reads `make_game.py` (52), on Windows,** passes apart from `test_cpp_preflight`, `test_linux_update` and `test_prepare_stall`, which fail the same way on a clean `main` here. - **Not run:** mooki000's gist fixture, which this machine's tool permissions wouldn't let me run. Its `guarded` variant is the same check this PR adds; a run with `--variant` pointed at this branch's commit would confirm it. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

github-sync added 1 commit

2026-11-12 06:22:42 +00:00

fix(build): the key is taken again after the lift only if its other inputs did not change a29018fa04

#2674 stores the key taken after the lift, so that the files the lift writes for the next one (image/hot_pieces.txt,
image/chunks.txt) are in it. Taken whole, that key also covered a translator file edited while the lift ran, and
certified C made from the old file under the new file's key: the next build skipped a lift it needed (mooki000's
fixture on #2674).

translate() now takes lift_key(planning=False), everything but those two files, as the lift starts and again after it,
and takes the key again only if that is unchanged. Otherwise the key from before stays, and the next build translates
again, as before #2674. tests/test_lift_cleanup_error.py adds the case of an input changed during the lift, and
tests/test_lift_cache.py checks the order and what planning=False leaves out and keeps.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

github-sync referenced this pull request

2026-11-12 06:24:12 +00:00

fix(build): a new work folder's second build no longer translates the game again #2674

github-sync commented

2026-11-12 06:24:25 +00:00

Author

Owner

@mooki000 commented on GitHub · 2026-11-12 06:19 UTC · original

The run you asked for on #2674, of the fixture with its two commit ids set to current main and this head (Linux, Python 3.12, no game, no compiler):

	a translator file edited during the lift	only the lift's own planning files change
main `c544828f` (has #2674)	fails: the old output is certified, the next build would skip	passes
this head `a29018fa`	passes: not certified, the next build lifts again (1 lift)	passes: the next build skips (0 lifts), "the game was translated with these tools before"

So the window is closed and #2674's gain is kept. Your own two test files pass here on this head (tests/test_lift_cache.py, tests/test_lift_cleanup_error.py), and with main's nx2/make_game.py put under them tests/test_lift_cache.py and tests/test_lift_cleanup_error.py fail, as you say.

One case neither version nor the code before #2674 sees, for the record and not for this pull request: a file changed and changed back during one lift (a branch switched and switched back while it runs) compares equal before and after.

Not tested: Windows, and a real lift (the fixture stands in for the lift and the split; everything around them is the repository's code).

> [@mooki000](https://github.com/mooki000) commented on GitHub · 2026-11-12 06:19 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2686#issuecomment-6816336791) The run you asked for on #2674, of [the fixture](https://gist.githubusercontent.com/mooki000/b86bd3f35969803fb032f71f035844f8/raw/efba25623bdaebc9ba2bd4101513a198d9b7e782/hookshot-2674-test_lift_key_input_race.py) with its two commit ids set to current main and this head (Linux, Python 3.12, no game, no compiler): | | a translator file edited during the lift | only the lift's own planning files change | |---|---|---| | main `c544828f` (has #2674) | **fails**: the old output is certified, the next build would skip | passes | | this head `a29018fa` | **passes**: not certified, the next build lifts again (1 lift) | passes: the next build skips (0 lifts), "the game was translated with these tools before" | So the window is closed and #2674's gain is kept. Your own two test files pass here on this head (`tests/test_lift_cache.py`, `tests/test_lift_cleanup_error.py`), and with main's `nx2/make_game.py` put under them `tests/test_lift_cache.py` and `tests/test_lift_cleanup_error.py` fail, as you say. One case neither version nor the code before #2674 sees, for the record and not for this pull request: a file changed and changed back during one lift (a branch switched and switched back while it runs) compares equal before and after. Not tested: Windows, and a real lift (the fixture stands in for the lift and the split; everything around them is the repository's code).

github-sync referenced this pull request from a commit

2026-11-12 07:27:35 +00:00

fix(build): the lift key is taken before the comparison that keeps it

github-sync added 1 commit

2026-11-12 07:27:35 +00:00

fix(build): the lift key is taken before the comparison that keeps it 54e2652632

The key was taken after lift_key(planning=False) was compared, so a translator file edited between the two would still
stamp C from the old file with the new key (SimonKimDev on #2686). It is now taken first and kept only when the
comparison after it holds: an edit before it is caught by the comparison, and one after it is in no key that is kept.
tests/test_lift_cache.py checks that order, and the cleanup test's changed-input case sees the key taken and not kept.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

github-sync referenced this pull request

2026-11-12 07:27:38 +00:00

perf(build): keep unchanged button-prompt assets in source builds #2698

github-sync commented

2026-11-12 07:30:28 +00:00

Author

Owner

@SimonKimDev reviewed on GitHub · 2026-11-12 06:40 UTC · original

Small window left in the check: the full key is taken after the comparison, so a translator file edited between the second lift_key(..., planning=False) and lift_key(exefs, work, env) still stamps C from the old translator with the new key.

Taking the full key first closes it:

after = lift_key(exefs, work, env)
if lift_key(exefs, work, env, planning=False) == inputs:
    key = after

An edit before after is caught by the comparison; an edit after it doesn't matter, since after already hashed the old file.

> [@SimonKimDev](https://github.com/SimonKimDev) reviewed on GitHub · 2026-11-12 06:40 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2686#pullrequestreview-5870224715) Small window left in the check: the full key is taken after the comparison, so a translator file edited between the second `lift_key(..., planning=False)` and `lift_key(exefs, work, env)` still stamps C from the old translator with the new key. Taking the full key first closes it: ```python after = lift_key(exefs, work, env) if lift_key(exefs, work, env, planning=False) == inputs: key = after ``` An edit before `after` is caught by the comparison; an edit after it doesn't matter, since `after` already hashed the old file.

github-sync commented

2026-11-12 07:30:28 +00:00

Author

Owner

@jordanella commented on GitHub · 2026-11-12 07:09 UTC · original

@SimonKimDev right, thanks: the order left exactly that gap. 54e26526 takes the full key first and keeps it only if the planning=False comparison after it holds, as you wrote; test_lift_cache.py now checks that order, and fails on the previous commit.

@mooki000 thank you for running the fixture on both sides; that's the confirmation I couldn't get here. Noted on a file changed and changed back within one lift (a branch switched and back): no snapshot of hashes sees that. It's out of this PR, as you say.

> [@jordanella](https://github.com/jordanella) commented on GitHub · 2026-11-12 07:09 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2686#issuecomment-6817219637) @SimonKimDev right, thanks: the order left exactly that gap. 54e26526 takes the full key first and keeps it only if the `planning=False` comparison after it holds, as you wrote; `test_lift_cache.py` now checks that order, and fails on the previous commit. @mooki000 thank you for running the fixture on both sides; that's the confirmation I couldn't get here. Noted on a file changed and changed back within one lift (a branch switched and back): no snapshot of hashes sees that. It's out of this PR, as you say.

github-sync commented

2026-11-12 07:30:28 +00:00

Author

Owner

@mooki000 commented on GitHub · 2026-11-12 07:12 UTC · original

The same fixture on 54e26526: unchanged from a29018fa. A translator file edited during the lift is not certified and the next build lifts again (1 lift); with only the planning files changing, the next build skips (0 lifts); main c544828f still fails the first case.

> [@mooki000](https://github.com/mooki000) commented on GitHub · 2026-11-12 07:12 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2686#issuecomment-6817263080) The same fixture on `54e26526`: unchanged from `a29018fa`. A translator file edited during the lift is not certified and the next build lifts again (1 lift); with only the planning files changing, the next build skips (0 lifts); main `c544828f` still fails the first case.

github-sync referenced this pull request from a commit

2026-11-12 08:30:49 +00:00

perf(build): keep unchanged button-prompt assets in source builds (#2698)

github-sync referenced this pull request

2026-11-12 13:38:20 +00:00

fix(build): check Linux prerequisites before reading the dump #2793

github-sync referenced this pull request

2026-11-12 13:40:11 +00:00

docs: a proposal for a faster development loop #2636

github-sync commented

2026-11-12 14:44:15 +00:00

Author

Owner

@Joey-Magnus approved on GitHub · 2026-11-12 14:34 UTC · original

SimonKimDev's ordering point is fixed in 54e26526. after = lift_key(...) is now taken before the planning=False comparison at nx2/make_game.py:2190, and test_lift_cache.py checks that order. mooki000's fixture on the new head confirms both cases: an edit made during the lift is not certified, and a build where only the planning files changed still skips the lift. I merged this into current main. tests/test_lift_cleanup_error.py passes (9 tests, including the new "input changed during the lift" case). tests/test_lift_cache.py has the same single failure here as on main (numpy isn't installed for the lift import check), and all the new checks pass. The fallback is the conservative one: if anything changed during the lift, the pre-lift key stays and the next build lifts again. That costs a few minutes at worst, never a stale build. An edit reverted during the lift (A to B to A) can't be seen by a content hash, but the C would be from A anyway, so that's fine. It merges cleanly with #2693, which also edits make_game.py.

> [@Joey-Magnus](https://github.com/Joey-Magnus) approved on GitHub · 2026-11-12 14:34 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2686#pullrequestreview-5876233560) SimonKimDev's ordering point is fixed in 54e26526. `after = lift_key(...)` is now taken before the `planning=False` comparison at `nx2/make_game.py:2190`, and `test_lift_cache.py` checks that order. mooki000's fixture on the new head confirms both cases: an edit made during the lift is not certified, and a build where only the planning files changed still skips the lift. I merged this into current main. `tests/test_lift_cleanup_error.py` passes (9 tests, including the new "input changed during the lift" case). `tests/test_lift_cache.py` has the same single failure here as on main (numpy isn't installed for the lift import check), and all the new checks pass. The fallback is the conservative one: if anything changed during the lift, the pre-lift key stays and the next build lifts again. That costs a few minutes at worst, never a stale build. An edit reverted during the lift (A to B to A) can't be seen by a content hash, but the C would be from A anyway, so that's fine. It merges cleanly with #2693, which also edits `make_game.py`.

github-sync referenced this pull request

2026-11-12 14:44:19 +00:00

feat(build): build_times.json and the build's last lines say where the time went #2693

github-sync referenced this pull request

2026-11-12 16:55:29 +00:00

docs: a proposal for a faster development loop #2636

github-sync referenced this pull request

2026-11-12 16:55:58 +00:00

fix(lift): write a split chunk's parts before the chunk itself #2751

github-sync referenced this pull request from a commit

2026-11-12 17:57:18 +00:00

fix(build): the lift key is taken again only if the lift's other inputs did not change (#2686)