Skip to content

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

Merged
hckrman101 merged 2 commits into
NotRexed:mainfrom
jordanella:fix/lift-key-after-lift
Nov 12, 2026
Merged

hckrman101 merged 2 commits into
NotRexed:mainfrom
jordanella:fix/lift-key-after-lift

Conversation

@jordanella

Copy link
Copy Markdown
Contributor

In a new work folder (a fresh worktree, or a player's new install), the second build translates the whole game again even though nothing changed: 319 s for a no-op rebuild on Windows, 290 s of it the lift. For players, the first update after a new install doesn't get to keep the translation (#2324).

Cause

lift_key hashes image/hot_pieces.txt, split_hot's kept pieces, and the first lift in a work folder is what writes that file. The key was taken before that lift and stored after it. So the next build, which now finds hot_pieces.txt, computes another key and translates again. After that the key is stable, which is why the problem only shows once per folder.

Change

  • translate() takes the key again after the lift and split_hot, from the files the next build will read, and stores that one. keep_translation uses it too (nonlocal key).
  • A lift run from those inputs (the pieces this lift wrote, the boundaries it kept) writes the same C. If they ever differ, the build relifts, as it does today.

Testing

  • tests/test_lift_cache.py:
    • A new check fails without the change and passes with it: the key is taken again after split_hot, and a fresh folder's key from before its first lift isn't the next build's.
    • The test also read the lift's module list by whitespace, so it failed in any checkout whose path has a space in it (here E:\Hobby Development\...). It now reads by lines. CI's paths have no spaces, so CI was not affected.
  • tests/test_lift_cleanup_error.py runs the fresh-lift branch with stubs; it now stands in for lift_key and expects lift → key taken again → key written. Against main's make_game.py it fails, with the change it passes.
  • End to end on Windows (Ryzen 9 3900X, NVMe, main at 0ec31ff plus this change): in a work folder with no hot_pieces.txt and no key, the first build took 315 s (the lift is needed) and the second 14 s, skipping the lift. Without the change, the second took 319 s.
  • Full run_all.py on Windows: every check that reads make_game.py (52) passes, apart from test_cpp_preflight, test_linux_update and test_prepare_stall, which fail the same way on a clean main here.

Not tested

A player's update through the Builder end to end. It takes the same path in make_game.py (keep_translation, then kept_before with this key), but I haven't run it through the Builder.

Part of the dev-loop proposal's Phase 0 (#2636).

🤖 Generated with Claude Code

@mooki000

Copy link
Copy Markdown
Contributor

One case to close before this merges; the fix itself does what it says (the second build in a new work folder no longer translates again).

The self-produced hot_pieces.txt fix is sound, but recomputing the whole key at 7a561e39:nx2/make_game.py:2181–2182 can certify output made from an older tracked input.
I reproduced this without a game: real production key/stamp/cache-predicate functions and the actual translate() AST, with tiny authored files and stand-ins only for the lift/split children (the fixture; it reads both commits from a clone).

  1. The child reads the old synthetic nx2/lift.py input and emits OLD output.
  2. While that child “runs”, the fixture changes that non-self input; the child also writes chunks.txt/hot_pieces.txt normally.
  3. The parent stamps its output, then a second build asks the real lifted_before predicate about the current inputs.
    Observed Linux/Python 3.12 fixture output (next_lift counts child calls in that second build):
base 87991bc8: old_output_certified=False, next_lift=1  PASS
head 7a561e39: old_output_certified=True,  next_lift=0  FAIL (exit 1)
guarded head: old_output_certified=False, next_lift=1  PASS (first lift refused)

The base stamped the pre-lift key, so the changed input safely forced the next lift; this PR stamps the changed-input key over OLD output.
An in-memory observation comparing non-self inputs before/after and refusing to stamp when they change fixes this case; the self-file-only control still skips the second lift, preserving your intended improvement.
Please refresh the self-produced planning inputs only after confirming the remaining inputs are unchanged, or use an immutable input snapshot.
This window requires a tracked input to change during a lift—for example an edit while its minutes of work run—not an ordinary unchanged build; no compiler, game or GPU was used.

@hckrman101 hckrman101 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is a clean fix and the cause is explained well.

I checked the parts that have to hold for it to be safe:

  • The new key matches what the next build computes. lift_key hashes image/hot_pieces.txt, and split_hot writes it during the lift. The cleanup step removes sa and probe but leaves image/, so the next build hashes the same file. build.py writes its kept list header from .lift_key (lift_key_of), which now holds the new key, and keep_translation checks against the same key through nonlocal key. So the player's kept_before path agrees with the dev path.
  • Skipping the lift on that key is correct. With prev equal to the pieces this lift just saved and the same bodies, sticky_split puts every function back in its own piece. Its largest cost then equals the fresh split's, so the sticky split wins, and each piece is written as sorted(names). The relift would write the same C, so the claim in the description holds. A stale hot_pieces.txt that split_hot dropped as out of balance is handled too: the key now picks up the rewritten file.
  • The second pass of the cut-short loop still relifts. The key file is removed first, so a key changed by the first pass can't match anything.
  • The tests. test_lift_cleanup_error compiles only the if lifted_before branch, so key = lift_key(...) rebinds the key in the test's namespace, and the stub sees exactly one rekey between lift and key. The splitlines() fix in test_lift_cache is a real fix for checkouts whose path has a space in it.

Re-hashing costs one extra lift_key call (an import walk and hashes of a few files) after a lift that takes minutes anyway, so that's fine.

CI: when I looked, the Python source checks on Windows (both parts) and Ubuntu part 2/2 were still running, along with the Builder package and DLSS/FSR link check jobs. Everything finished so far passes. Good to merge once those are green (Windows part 2/2 currently fails on main on test_psychov35/test_pcache_header_read; those failures aren't from this PR).

@hckrman101
hckrman101 merged commit c544828 into NotRexed:main Nov 12, 2026
14 checks passed
@jordanella

Copy link
Copy Markdown
Contributor Author

Thanks, @mooki000: you're right, and I missed that window. Taking the whole key after the lift lets a translator file edited mid-lift vouch for C made from the old one. #2686 does what you suggested. It snapshots everything but the lift's own planning files (lift_key(..., planning=False)) as the lift starts, compares after it, and takes the key again only if nothing else changed; otherwise the pre-lift key stays and the next build relifts, as on the base. test_lift_cleanup_error.py has the mid-lift edit as a case, and it fails on main. I couldn't run your gist fixture here (this machine's tool permissions), so a run of it against #2686's head would be very welcome.

jordanella and others added 2 commits November 11, 2026 22:29
… game again

The lift key hashes image/hot_pieces.txt, split_hot's kept pieces, which the first lift in a work folder writes. The key
was taken before that lift and stored after it, so in a new work folder (a fresh worktree, a new install) the next build
never matched it and translated the whole game again for nothing: 319 s for a no-op rebuild on Windows, 290 s of it the
lift. For players, the first update after a new install did not keep the translation (NotRexed#2324).

translate() now takes the key again after the lift and split_hot, from what the next build reads, and stores that one;
keep_translation uses it too. tests/test_lift_cache.py checks the order, and that the key before a first lift is not the
next build's. It also reads the lift's module list by lines, not by whitespace, so it passes in a checkout whose path has
a space in it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
tests/test_lift_cleanup_error.py runs make_game's fresh-lift branch with stubs; it now stands in for lift_key too and
expects the order the previous commit makes: the lift, the key taken again, the key written.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
hckrman101 pushed a commit that referenced this pull request Nov 12, 2026
)

**What this is for:** contributors rebuilding from source should not
spend another minute drawing the same button prompts. **What is
measured:** on an Intel Core i9-13900K running Arch Linux, the finish
stage at main `c544828f` took 67.707 and 66.898 seconds; two unchanged
runs took 0.603 and 0.651 seconds, and the final head took 0.616
seconds, clearing the run-time files as well. Other work was running
throughout. **Not measured:** Windows, a packaged Builder update, a
complete compile/lift, or gameplay performance.

Refs #2636.

## Problem

`nx2/make_game.py` removes the game folder's `prompts` directory and
runs `prompt_assets.py` on every build when fontTools is available. Most
source edits do not change anything those assets depend on.

This avoids drawing unchanged assets; it does not make the drawing
itself faster. #2636's A1 plans to measure the finish stage, but its
A1–A10 items do not already propose this prompt cache. It complements
#2674 and #2675, which address unnecessary retranslation rather than
prompt generation.

## What changed

- **`nx2/make_game.py`:** source-checkout builds request incremental
prompt generation. Packaged builders still regenerate every time, using
the finish stage's existing launcher distinction. `RX_RELEASE` cannot
distinguish them: make_game sets it for source builds too. Missing
fontTools still removes the replacement prompts, keeping the existing
fallback to the game's icons.
- **`nx2/prompt_assets.py`:** fingerprint the inputs before generating,
draw into a fresh sibling directory, and record the successful outputs
only if a second fingerprint still matches. Reuse requires both matching
inputs and every recorded output's size and SHA-256. **A failed
generation still removes the prompts, as before**, so the game shows its
own icons. An interrupted builder can leave the previous complete set
without a stamp; the next build regenerates it. The drawing, encoding
and asset formats are unchanged.
- **`tests/test_prompt_skip.py`:** game-free fixtures run the actual
finish-stage prompt call and production cache/publication functions,
with small authored files and a stand-in for expensive drawing.
- **`docs/development.md`:** explain reuse, its limits, and
`HOOKSHOT_FORCE_PROMPTS=1`. The force switch follows `env_on`: unset,
empty, `0`, `false`, `off` and `no` are off, ignoring case and
surrounding spaces.

### What prevents a wrong skip

The input record covers:

- The four prompt source modules, including controller, keyboard and
binding tables.
- The game's icon font and optional bold label font, compression
dictionary, common texture/layout archives, and matching resource-size
table names and contents.
- The repository's controller and keyboard fonts, and added, changed or
removed PNG overrides.
- Python/platform; fontTools, Pillow, ASTC encoder, texture decoder,
NumPy, zstandard and archspec availability and versions; Pillow's native
library versions and the selected ASTC backend.
- Generation parameters and relevant library switches, including
`SOURCE_DATE_EPOCH`, `FREETYPE_PROPERTIES` and the encoder's custom CPU
definitions.

Languages and saved bindings are not build inputs here: the generator
uses its source tables, and the runtime applies the selected controls.

The output record includes files **and directories**: the keyboard
directory itself is a runtime marker. Legitimately absent optional
outputs stay absent; a missing, truncated or same-size-corrupted
recorded file forces regeneration. The runtime's own compressed copies
and control-specific keys are excluded before hashing, not mistaken for
generator output. **The files the game derives at run time are still
cleared by every build**, including an unchanged hit: their own keys do
not cover changes to `host/prompts.c`. Only the known runtime filenames
are removed; if one cannot be cleared, the build regenerates instead of
keeping on doubt.

An unreadable input cannot authorize reuse. The log gives the reason,
even if a forced regeneration subsequently fails. A source edited during
generation leaves no stamp, addressing the stale-input certification
problem discussed in [#2674's
review](#2674 (comment)).

## How it was tested

2026-11-12; Arch Linux, kernel `7.2.15-1-omarchy`, x86-64, Intel Core
i9-13900K, 32 logical CPUs, Python 3.12.16. Base `c544828ff`; final head
`2ec89de8`. No game or GPU program was launched for these checks.

### Finish-stage measurements

These execute the real finish-stage statements through completion,
including binary/image copies, the unchanged shader-list check,
pipeline-list work and prompt generation, in a disposable copy of an
existing built game folder. Compilation, translation and shader
preparation are not included. They are **not whole rebuild times**.

All optional prompt packages were present: fontTools 4.68.0, Pillow
13.0.0, astc-encoder-py 0.1.12, texture2ddecoder 1.0.6, archspec 0.2.7,
NumPy 2.5.4 and zstandard 0.25.0.

| Run | Source | Finish time | 1-minute load before → after | Top other
work |
|---|---|---:|---:|---|
| Before 1 | `c544828f` | 67.707 s | 7.13 → 7.40 | Existing game 316 →
315% CPU; file indexing 66 → 79%; relay check also active |
| Before 2 | `c544828f` | 66.898 s | 7.06 → 8.24 | Existing game 315 →
318%; terminal process about 41% |
| First generation with cache | `63d5c965` | 66.400 s | 8.24 → 8.19 |
Existing game 318 → 320%; terminal process about 42% |
| Unchanged 1 | `63d5c965` | 0.603 s | 8.19 → 8.19 | Existing game 320%;
terminal process about 42% |
| Unchanged 2 | `63d5c965` | 0.651 s | 8.19 → 8.19 | Existing game 320%;
terminal process about 42% |
| Changed table source | `681208dd` | 63.147 s | 6.99 → 6.95 | Existing
game 320 → 321%; terminal process about 42% |
| Regenerate, final head | `2ec89de8` | 65.007 s | 5.77 → 5.86 |
Existing game 317 → 316%; terminal process about 43% |
| Unchanged, final head, with run-time files present | `2ec89de8` |
0.616 s | 5.79 → 5.79 | Existing game 316%; terminal process about 43% |

Other work was running; none was a quiet measurement. CPU percentages
are lifetime averages from snapshots, not interval measurements. The
first generation is included to show that this is a skip, not a faster
drawing algorithm.

The unchanged runs print:

```text
prompts: unchanged, kept
```

Adding a harmless comment to the button-table source forces a fresh
generation:

```text
prompts: regenerated (table changed)
```

The comment was removed afterwards. A timestamp-only touch does not
regenerate: the key checks content. An additional run on the final head
verifies an unchanged hit with all six runtime-derived files present:
all six are removed, while the 16 generated originals and the stamp
retain their sizes and SHA-256 values. The focused fixture also compares
the retained originals byte for byte. The final timing rows above
include that cleanup.

**Asset parity:** all 16 regenerated files, 274,704,680 bytes, compare
byte for byte with a full generation by main `c544828f`. Only the new
stamp is excluded. Both sides used the same `SOURCE_DATE_EPOCH` because
fontTools otherwise writes the current time into the OpenType header.
This comparison setting is not introduced into normal builds. A
development cache hit retains the earlier font timestamp; packaged
builds retain their old full-generation behavior.

### Game-free checks

- **23 focused tests pass** on the final change. They cover unchanged
reuse, every input kind, package versions/presence, switch/parameter
changes, missing/truncated/same-size-corrupted output, empty
directories, clearing runtime derivatives while retaining byte-identical
originals, regeneration when runtime cleanup fails, interruption,
mid-generation edits, unreadable input, force-switch parsing, incomplete
output, publication failure, and the packaged/no-fontTools paths. A
non-zero generator result removes prompts for both source and packaged
builds; interruption before that result leaves the old set unstamped.
- The same unchanged-build test against main's finish stage fails
behaviorally: `2 != 1: unchanged finish must not draw the prompts
twice`.
- Removing each of **18 guards/key components** in memory makes its
corresponding behavior test fail on the final change. Three later
corrections also fail against the earlier version of this patch:
returned failure retained old prompts in both build paths, runtime files
survived a hit, and an uncleared runtime file did not force
regeneration. The unreadable-input diagnostic has a separate
before-fails/after-passes proof.
- The whole suite one commit earlier, at `f20ea58b` (before a failed
generation was made to remove the prompts again and the run-time files
to be cleared), `python -I -B tests/run_all.py --jobs 8`:

  ```text
  472 passed, 0 failed, 20 skipped in 133 s (linux, 8 at once)
  ```

Load was 7.37 → 19.48. The first full run, at 32 checks at once, was
**471 passed, 1 failed, 20 skipped**: a clang++ child compiling
`texture_pass.cpp` for `nx2/test_depth_sample_types.py` died with
SIGKILL. That fixture and its input trees are unchanged from main. No
kernel OOM line was found, so the cause is not established. The complete
run with eight workers passes, including that check. The failed run is
retained, not counted as a pass. On the final head `2ec89de8` the
focused checks, the baseline failure and all guard mutations were rerun,
and the whole suite once more with eight workers: **471 passed, 1
failed, 20 skipped in 136 s** (load 11.14 → 22.62). The one failure is
`tests/test_release_scripts.py`,
`test_nightly_two_runs_at_once_end_at_the_newer_commit`, the
intermittent race reported in #2626; this change touches none of its
files.
- No core dumps were recorded. Nothing generated from the game is in
this change.

## Not tested

- Windows filesystem behavior or a Windows build.
- The packaged Windows Builder or Linux AppImage build/update end to
end. Their always-regenerate selection is covered by a game-free test
only.
- A full compile/lift or a new gameplay run, other CPUs, or
gameplay/frame-rate benefits.

No screenshots: the regenerated asset bytes match, and no visual change
is intended.

## Risks

The input list must grow when the generator gains new file dependencies
or outputs outside the prompt directory. A regression flags new local
module imports, but it does not replace reviewing new file reads.
Generation temporarily needs room for both old and new assets.
Publication uses completed directories and rollback on a rename error;
interruption cannot create a stamped half-set, though a process killed
between the two directory renames may require regeneration on the next
build.

## Overlaps

The open-PR scan found 47 PRs touching one of these files. The actual
conflict hunks were read:

- **#2151:** adjacent item-button generation conflicts with the
finish-stage prompt call/error branch. Preserve both steps and account
for added prompt dependencies.
- **#2261:** substantive overlap. It adds `--wheel-icons`, another input
folder and output outside `prompts`. Keep that work outside this skip or
extend the input/output proof and staging; accepting both hunks
mechanically is not enough.
- #1872, #1958, #2304, #2317, #2430, #2465, #2474, #2478, #2608, #2673,
#2675 and #2686 share code files but merged cleanly in the scan. That is
not proof that any newly added generator dependency is covered.

Assisted-by: GPT-6 Astra
Co-Authored-By: GPT-6 Astra <noreply@openai.com>

Co-authored-by: GPT-6 Astra <noreply@openai.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants