From 29903233b5978b8136b5af8a72501f05d0c806b1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 10 Jun 2026 14:16:11 +0000 Subject: [PATCH 01/10] Add ROM gitignore protection, CI workflow, unit tests; fix Space.write overflow corruption Priority 1 items from the v1.4.3 code review: - .gitignore: exclude *.smc/*.sfc ROM files (legal requirement) and generated test artifacts in tests/ - CI (.github/workflows/ci.yml): syntax check, unit tests, and wc.py -h smoke test on Python 3.9-3.13, plus a guard job that fails if any ROM-like or oversized file is ever committed - tests/: new unit test suite (65 tests) covering the ROM-independent core: heap allocation/free/reserve, label and branch encoding, Space write/clear/conflict handling, compression round-trips, flatten/intersection/weighted_random/shuffle_if, seeding, ROM validation, and CLI parsing - memory/space.py: validate bounds BEFORE writing in Space.write() so an overflowing write can no longer corrupt bytes beyond the end of its space; replace bare except clauses with except TypeError - memory/errors.py: new RomSpaceError (subclasses MemoryError for backward compatibility) raised by Space.write() and Heap.allocate() instead of the builtin MemoryError - agents.md / llms.md: updated error signature and test documentation No RNG calls are added, removed, or reordered: seed output is unchanged. https://claude.ai/code/session_01QnF26DB2TYi1hpJE8L6EJr --- .github/workflows/ci.yml | 47 +++++++++++ .gitignore | 12 ++- agents.md | 14 +++- llms.md | 2 + memory/errors.py | 8 ++ memory/heap.py | 4 +- memory/space.py | 16 ++-- tests/__init__.py | 0 tests/test_cli.py | 23 ++++++ tests/test_heap.py | 94 ++++++++++++++++++++++ tests/test_label.py | 71 ++++++++++++++++ tests/test_seed.py | 51 ++++++++++++ tests/test_space.py | 152 +++++++++++++++++++++++++++++++++++ tests/test_utils.py | 101 +++++++++++++++++++++++ tests/test_valid_rom_file.py | 25 ++++++ 15 files changed, 610 insertions(+), 10 deletions(-) create mode 100644 .github/workflows/ci.yml create mode 100644 memory/errors.py create mode 100644 tests/__init__.py create mode 100644 tests/test_cli.py create mode 100644 tests/test_heap.py create mode 100644 tests/test_label.py create mode 100644 tests/test_seed.py create mode 100644 tests/test_space.py create mode 100644 tests/test_utils.py create mode 100644 tests/test_valid_rom_file.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 00000000..0b8f7080 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,47 @@ +name: CI + +on: + push: + pull_request: + +jobs: + rom-guard: + name: ROM safety guard + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Fail if ROM or oversized files are tracked + run: | + set -e + rom_files=$(git ls-files | grep -iE '\.(smc|sfc)$' || true) + if [ -n "$rom_files" ]; then + echo "::error::ROM files must never be committed (legal requirement):" + echo "$rom_files" + exit 1 + fi + # largest legitimate source file is ~56KB; anything over 512KB is suspect + big_files=$(git ls-files -z | xargs -0 du -b 2>/dev/null | awk '$1 > 524288 {print $2 " (" $1 " bytes)"}') + if [ -n "$big_files" ]; then + echo "::error::Unexpectedly large files tracked (possible ROM or binary data):" + echo "$big_files" + exit 1 + fi + + test: + name: Tests (Python ${{ matrix.python-version }}) + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python-version: ["3.9", "3.10", "3.11", "3.12", "3.13"] + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + - name: Syntax check all modules + run: python -m compileall -q . + - name: Run unit tests + run: python -m unittest discover -s tests -v + - name: CLI smoke test + run: python wc.py -h diff --git a/.gitignore b/.gitignore index eaa59db1..191b6831 100644 --- a/.gitignore +++ b/.gitignore @@ -3,4 +3,14 @@ __pycache__/ $py.class *-objective.json *-flag.json -wip/ \ No newline at end of file +wip/ + +# ROM files must never be committed (legal requirement) +*.smc +*.sfc +*.SMC +*.SFC + +# generated test artifacts (seed logs, manifests) - see agents.md "Test Data Isolation" +tests/*.txt +tests/*.json diff --git a/agents.md b/agents.md index 76e4e727..6d60035b 100644 --- a/agents.md +++ b/agents.md @@ -24,9 +24,16 @@ To verify ROM modifications and successfully run a seed generation test, a valid ### 1.3 Test Data Isolation > [!IMPORTANT] -> **TEST DATA DIRECTORY RULE**: All test output data (including modified test ROMs, debug log text files, or API metadata manifests generated during your test runs) **MUST** be isolated and placed inside a `tests` directory in the workspace root (e.g., `tests/test_output.smc`, `tests/test_output.txt`). +> **TEST DATA DIRECTORY RULE**: All test output data (including modified test ROMs, debug log text files, or API metadata manifests generated during your test runs) **MUST** be isolated and placed inside the `tests` directory in the workspace root (e.g., `tests/test_output.smc`, `tests/test_output.txt`). > - Do not write test output files directly to the root directory. -> - Ensure the `tests` directory is created before running commands (e.g. `mkdir -Force tests`). +> - The `tests` directory also contains the committed unit test suite (`tests/test_*.py`); generated artifacts there are excluded from git via `.gitignore` (`*.smc`, `*.sfc`, `tests/*.txt`, `tests/*.json`). + +### 1.4 Unit Tests +The `tests/` directory contains a unit test suite for the ROM-independent logic (memory allocation, label/branch encoding, compression, seeding, CLI parsing). It requires no ROM file: +```powershell +python3 -m unittest discover -s tests -v +``` +Run this after any change to `memory/`, `utils/`, `seed.py`, or `valid_rom_file.py`, and add tests for new pure-Python logic. The same suite runs in CI (`.github/workflows/ci.yml`) on every push and pull request, along with `python -m compileall`, `wc.py -h`, and a guard that fails if any ROM-like or oversized file is ever committed. --- @@ -64,8 +71,9 @@ During development, agents commonly trigger three specific errors. Here is how t ### 3.1 Memory Overflow / Bank Exhaustion **Error Signature**: ```text -MemoryError: Not enough room in space "custom event toggle": Next (0xc0f124) > End (0xc0f100). Diff: 36 +memory.errors.RomSpaceError: Not enough room in space "custom event toggle": Next (0xc0f124) > End (0xc0f100). Diff: 36 ``` +(`RomSpaceError` subclasses `MemoryError`, so older code or docs referring to `MemoryError` still apply. The overflow is detected *before* any bytes are written, so the ROM buffer is never corrupted by an overflowing write.) **Cause**: You have written more bytes of assembly instructions or static data than the target `Reserve` range fits, or have exceeded the size of a dynamically requested `Allocate` block in that bank. **Resolution**: 1. Check the size parameters in your dynamic allocation: `Allocate(Bank.C0, size, "description")`. Increase `size` to support your assembly array length. diff --git a/llms.md b/llms.md index c03eea55..24ab193f 100644 --- a/llms.md +++ b/llms.md @@ -183,6 +183,8 @@ All generated test data, output ROMs, log text files, and metadata manifests gen - Output test ROM: `tests/test_output.smc` - Output test Log: `tests/test_output.txt` - Manifest file: `tests/test_manifest.json` +- **Unit Tests**: `tests/` also contains the committed unit test suite (`tests/test_*.py`) covering ROM-independent logic (memory allocation, label/branch encoding, compression, seeding, CLI parsing). Run it with `python3 -m unittest discover -s tests -v`; it requires no ROM file. CI (`.github/workflows/ci.yml`) runs it on every push/PR, plus a guard that rejects committed ROM-like files. Generated artifacts in `tests/` are gitignored (`*.smc`, `*.sfc`, `tests/*.txt`, `tests/*.json`); test source files are tracked. +- **Memory Overflow Errors**: Space overflows raise `memory.errors.RomSpaceError` (a `MemoryError` subclass). Bounds are validated *before* writing, so an overflowing `space.write(...)` never corrupts adjacent ROM bytes. --- diff --git a/memory/errors.py b/memory/errors.py new file mode 100644 index 00000000..c98b0515 --- /dev/null +++ b/memory/errors.py @@ -0,0 +1,8 @@ +# RomSpaceError subclasses MemoryError because these failures were historically +# raised as MemoryError; existing callers/tools catching MemoryError still work +class RomSpaceError(MemoryError): + """Raised when data does not fit in the requested/reserved ROM space. + + Common causes and resolutions are documented in agents.md under + "Memory Overflow / Bank Exhaustion". + """ diff --git a/memory/heap.py b/memory/heap.py index 55460e52..2f56ab6a 100644 --- a/memory/heap.py +++ b/memory/heap.py @@ -1,3 +1,5 @@ +from memory.errors import RomSpaceError + class Block: def __init__(self, start, end): if start > end: @@ -53,7 +55,7 @@ def find_best_fit(size): block = find_best_fit(size) if block is None: - raise MemoryError(f"Unable to allocate block of size {size}") + raise RomSpaceError(f"Unable to allocate block of size {size}") start = block.start block.start += size diff --git a/memory/space.py b/memory/space.py index 5cc6c945..39eb51eb 100644 --- a/memory/space.py +++ b/memory/space.py @@ -1,6 +1,7 @@ from memory.rom import ROM from memory.heap import Heap from memory.label import Label, LabelPointer +from memory.errors import RomSpaceError from enum import IntEnum BANK_SIZE = 0x10000 @@ -80,16 +81,20 @@ def write(self, *values): values = self._invoke_callables(values) values = self._parse_labels(values) - self._next_address = Space.rom.set_bytes(self.next_address, values) - if(self.next_address - 1 > self.end_address): - raise MemoryError(f"Not enough room in space \"{self.description}\": Next (0x{self.next_address -1:x}) > End (0x{self.end_address:x}). Diff: {(self.next_address - 1) - (self.end_address)}") + # validate bounds before writing so an overflow cannot corrupt bytes + # beyond the end of this space + last_address = self.next_address + len(values) - 1 + if last_address > self.end_address: + raise RomSpaceError(f"Not enough room in space \"{self.description}\": Next (0x{last_address:x}) > End (0x{self.end_address:x}). Diff: {last_address - self.end_address}") + self._next_address = Space.rom.set_bytes(self.next_address, values) self._update_label_pointers() def clear(self, value): try: values = [value] * (len(self) // len(value)) - except: + except TypeError: + # value is a single int (no len()), not a sequence values = [value] * len(self) values = self._invoke_callables(values) @@ -177,7 +182,8 @@ def _parse_labels(self, values): new_values.append(value) try: index += len(value) - except: + except TypeError: + # value is a single byte/int (no len()) index += 1 return new_values diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/test_cli.py b/tests/test_cli.py new file mode 100644 index 00000000..bf8e0d5d --- /dev/null +++ b/tests/test_cli.py @@ -0,0 +1,23 @@ +import os +import subprocess +import sys +import unittest + +REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + +class TestCli(unittest.TestCase): + def test_help_exits_successfully(self): + # smoke test: importing args parses all 30+ flag modules, so -h + # exercises the entire argument interface without needing a ROM + result = subprocess.run( + [sys.executable, "wc.py", "-h"], + cwd = REPO_ROOT, + capture_output = True, + text = True, + timeout = 60, + ) + self.assertEqual(result.returncode, 0, msg = result.stderr) + self.assertIn("usage", result.stdout) + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_heap.py b/tests/test_heap.py new file mode 100644 index 00000000..374d1021 --- /dev/null +++ b/tests/test_heap.py @@ -0,0 +1,94 @@ +import unittest + +from memory.errors import RomSpaceError +from memory.heap import Block, Heap + +class TestBlock(unittest.TestCase): + def test_size_is_inclusive_of_both_ends(self): + self.assertEqual(Block(0, 0).size, 1) + self.assertEqual(Block(0, 9).size, 10) + + def test_swapped_bounds_are_normalized(self): + block = Block(9, 0) + self.assertEqual(block.start, 0) + self.assertEqual(block.end, 9) + self.assertEqual(block.size, 10) + +class TestHeap(unittest.TestCase): + def setUp(self): + self.heap = Heap() + + def test_allocate_from_empty_heap_raises(self): + with self.assertRaises(RomSpaceError): + self.heap.allocate(1) + + def test_allocate_raises_memory_error_for_backward_compatibility(self): + with self.assertRaises(MemoryError): + self.heap.allocate(1) + + def test_free_then_allocate(self): + self.heap.free(0x100, 0x1ff) + self.assertEqual(self.heap.available, 0x100) + + start = self.heap.allocate(0x10) + self.assertEqual(start, 0x100) + self.assertEqual(self.heap.available, 0xf0) + + def test_allocate_uses_best_fit_block(self): + self.heap.free(0, 99) # 100 byte block + self.heap.free(200, 219) # 20 byte block + + start = self.heap.allocate(20) + self.assertEqual(start, 200) # exact fit preferred over larger block + self.assertEqual(self.heap.available, 100) + + def test_allocate_more_than_largest_block_raises(self): + self.heap.free(0, 99) + self.heap.free(200, 219) + with self.assertRaises(RomSpaceError): + self.heap.allocate(101) # 120 bytes available but largest block is 100 + + def test_free_merges_adjacent_blocks(self): + self.heap.free(0, 9) + self.heap.free(10, 19) + self.assertEqual(self.heap.available, 20) + self.assertEqual(len(self.heap.blocks), 1) + self.assertEqual(self.heap.allocate(20), 0) + + def test_free_merges_overlapping_blocks(self): + self.heap.free(0, 14) + self.heap.free(10, 19) + self.assertEqual(self.heap.available, 20) + self.assertEqual(len(self.heap.blocks), 1) + + def test_free_inside_existing_block_changes_nothing(self): + self.heap.free(0, 99) + self.heap.free(40, 59) + self.assertEqual(self.heap.available, 100) + self.assertEqual(len(self.heap.blocks), 1) + + def test_reserve_splits_free_block(self): + self.heap.free(0, 99) + self.heap.reserve(40, 59) + self.assertEqual(self.heap.available, 80) + + # neither remaining block can hold more than 40 bytes + with self.assertRaises(RomSpaceError): + self.heap.allocate(41) + self.assertEqual(self.heap.allocate(40), 0) + self.assertEqual(self.heap.allocate(40), 60) + + def test_reserve_trims_overlapping_block(self): + self.heap.free(0, 99) + self.heap.reserve(50, 120) + self.assertEqual(self.heap.available, 50) + self.assertEqual(self.heap.allocate(50), 0) + + def test_reserve_consumes_fully_covered_block(self): + self.heap.free(10, 19) + self.heap.reserve(0, 29) + self.assertEqual(self.heap.available, 0) + self.assertEqual(len(self.heap.blocks), 0) + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_label.py b/tests/test_label.py new file mode 100644 index 00000000..53ec76b9 --- /dev/null +++ b/tests/test_label.py @@ -0,0 +1,71 @@ +import unittest + +from memory.label import Label, LabelPointer + +class TestLabelPointer(unittest.TestCase): + def _pointer(self, label_address, pointer_address, mode, offset = 0): + label = Label("TEST") + label.address = label_address + pointer = LabelPointer(label, pointer_address, mode) + pointer.offset = offset + return pointer + + def test_absolute(self): + pointer = self._pointer(0x1234, 0x100, LabelPointer.ABSOLUTE) + self.assertEqual(int(pointer), 0x1234) + + def test_absolute_with_offset(self): + pointer = self._pointer(0x1234, 0x100, LabelPointer.ABSOLUTE) + pointer = pointer + 4 + self.assertEqual(int(pointer), 0x1238) + pointer = pointer - 8 + self.assertEqual(int(pointer), 0x1230) + + def test_relative(self): + pointer = self._pointer(0x110, 0x100, LabelPointer.RELATIVE) + self.assertEqual(int(pointer), 0x10) + + pointer = self._pointer(0x100, 0x110, LabelPointer.RELATIVE) + self.assertEqual(int(pointer), -0x10) + + def test_absolute_relative(self): + pointer = self._pointer(0x100, 0x110, LabelPointer.ABSOLUTE_RELATIVE) + self.assertEqual(int(pointer), 0x10) + + # 65c816 branch offsets are relative to the program counter after the + # one-byte operand, so the encoded value is (distance - 1) mod 256 + def test_branch_relative_forward(self): + pointer = self._pointer(0x110, 0x100, LabelPointer.BRANCH_RELATIVE) + self.assertEqual(int(pointer), 0x0f) + + def test_branch_relative_backward(self): + pointer = self._pointer(0x100, 0x110, LabelPointer.BRANCH_RELATIVE) + self.assertEqual(int(pointer), 0xef) # two's complement of -0x11 + + def test_branch_relative_backward_minimal(self): + # branching to the previous byte: distance -1, encoded offset -2 (0xfe) + pointer = self._pointer(0x0ff, 0x100, LabelPointer.BRANCH_RELATIVE) + self.assertEqual(int(pointer), 0xfe) + + def test_branch_relative_out_of_range_raises(self): + pointer = self._pointer(0x200, 0x100, LabelPointer.BRANCH_RELATIVE) + with self.assertRaises(ValueError): + int(pointer) + + pointer = self._pointer(0x100, 0x200, LabelPointer.BRANCH_RELATIVE) + with self.assertRaises(ValueError): + int(pointer) + + def test_to_bytes_little_endian(self): + pointer = self._pointer(0x1234, 0x100, LabelPointer.ABSOLUTE) + self.assertEqual(pointer.to_bytes(2, "little"), b"\x34\x12") + + def test_comparisons_use_pointed_to_address(self): + pointer = self._pointer(0x1234, 0x100, LabelPointer.ABSOLUTE) + self.assertTrue(pointer < 0x1235) + self.assertTrue(pointer <= 0x1234) + self.assertTrue(pointer > 0x1233) + self.assertTrue(pointer >= 0x1234) + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_seed.py b/tests/test_seed.py new file mode 100644 index 00000000..fdda35f0 --- /dev/null +++ b/tests/test_seed.py @@ -0,0 +1,51 @@ +import random +import string +import unittest + +from seed import SEED_LENGTH, generate_seed, seed_rng + +class TestGenerateSeed(unittest.TestCase): + def test_length(self): + self.assertEqual(len(generate_seed()), SEED_LENGTH) + + def test_charset(self): + alpha_digits = set(string.ascii_lowercase + string.digits) + for _ in range(20): + self.assertTrue(set(generate_seed()) <= alpha_digits) + +class TestSeedRng(unittest.TestCase): + def setUp(self): + self._rng_state = random.getstate() + + def tearDown(self): + random.setstate(self._rng_state) + + def test_returns_given_seed(self): + self.assertEqual(seed_rng("abc123", "-i -o"), "abc123") + + def test_generates_seed_when_none_given(self): + seed = seed_rng(None, "") + self.assertEqual(len(seed), SEED_LENGTH) + + def test_deterministic_for_same_seed_and_flags(self): + # seed reproducibility is a core requirement: the same seed + flags + # must always produce the same random stream + seed_rng("abc123", "-flags") + first = [random.random() for _ in range(10)] + + seed_rng("abc123", "-flags") + second = [random.random() for _ in range(10)] + + self.assertEqual(first, second) + + def test_different_flags_produce_different_stream(self): + seed_rng("abc123", "-flags one") + first = random.random() + + seed_rng("abc123", "-flags two") + second = random.random() + + self.assertNotEqual(first, second) + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_space.py b/tests/test_space.py new file mode 100644 index 00000000..8421017b --- /dev/null +++ b/tests/test_space.py @@ -0,0 +1,152 @@ +import unittest + +import memory.space +from memory.errors import RomSpaceError +from memory.heap import Heap +from memory.space import Allocate, Bank, Free, Reserve, Space, Write + +class FakeRom: + """Stand-in for memory.rom.ROM backed by a plain list (no ROM file needed).""" + def __init__(self, size = 0x20000): + self.data = [0] * size + + def set_bytes(self, address, values): + self.data[address : address + len(values)] = values + return address + len(values) + + def get_bytes(self, address, count): + return self.data[address : address + count] + + def get_byte(self, address): + return self.data[address] + +class SpaceTestCase(unittest.TestCase): + """Space uses class-level shared state; isolate and restore it per test.""" + def setUp(self): + self._saved_rom = Space.rom + self._saved_heaps = Space.heaps + self._saved_spaces = Space.spaces + + self.rom = FakeRom() + Space.rom = self.rom + Space.heaps = { bank : Heap() for bank in Bank } + Space.spaces = [] + + def tearDown(self): + Space.rom = self._saved_rom + Space.heaps = self._saved_heaps + Space.spaces = self._saved_spaces + +class TestSpaceWrite(SpaceTestCase): + def test_write_within_bounds(self): + space = Space(0x100, 0x10f, "test space") + space.write(1, 2, 3) + self.assertEqual(self.rom.get_bytes(0x100, 3), [1, 2, 3]) + self.assertEqual(space.next_address, 0x103) + + def test_writes_are_sequential(self): + space = Space(0x100, 0x10f, "test space") + space.write([1, 2]) + space.write([3, 4]) + self.assertEqual(self.rom.get_bytes(0x100, 4), [1, 2, 3, 4]) + + def test_write_nested_values_are_flattened(self): + space = Space(0x100, 0x10f, "test space") + space.write([1, [2, 3]], (4, 5), b"\x06") + self.assertEqual(self.rom.get_bytes(0x100, 6), [1, 2, 3, 4, 5, 6]) + + def test_write_exactly_full(self): + space = Space(0x100, 0x103, "test space") + space.write([1, 2, 3, 4]) + self.assertEqual(self.rom.get_bytes(0x100, 4), [1, 2, 3, 4]) + + def test_write_overflow_raises(self): + space = Space(0x100, 0x103, "test space") + with self.assertRaises(RomSpaceError): + space.write([1, 2, 3, 4, 5]) + + def test_write_overflow_does_not_modify_rom(self): + # regression test: overflow used to be detected only after the + # out-of-bounds bytes had already been written to the rom buffer + self.rom.data[0x104] = 0xaa # byte just past the end of the space + space = Space(0x100, 0x103, "test space") + with self.assertRaises(RomSpaceError): + space.write([1, 2, 3, 4, 5]) + self.assertEqual(self.rom.get_byte(0x104), 0xaa) + self.assertEqual(self.rom.get_bytes(0x100, 4), [0, 0, 0, 0]) + + def test_overflow_error_is_a_memory_error_for_backward_compatibility(self): + space = Space(0x100, 0x103, "test space") + with self.assertRaises(MemoryError): + space.write([1, 2, 3, 4, 5]) + +class TestSpaceLabels(SpaceTestCase): + def test_backward_branch_label_resolution(self): + space = Space(0x100, 0x10f, "test space") + space.write( + "LOOP", + 0xea, 0xea, # 2 placeholder bytes + 0x80, space.branch_distance("LOOP"), # BRA LOOP + ) + # branch operands stay as LabelPointer objects in the rom buffer and + # resolve via __index__; distance -3, encoded as (distance - 1) mod 256 + self.assertEqual(int(self.rom.get_byte(0x103)), 0xfc) + + def test_duplicate_label_raises(self): + space = Space(0x100, 0x10f, "test space") + space.write("LABEL", 0xea) + with self.assertRaises(ValueError): + space.write("LABEL", 0xea) + +class TestSpaceClear(SpaceTestCase): + def test_clear_with_single_value(self): + space = Space(0x100, 0x103, "test space", clear_value = 0xff) + self.assertEqual(self.rom.get_bytes(0x100, 4), [0xff] * 4) + + def test_clear_with_instruction_value(self): + # multi-byte clear values are callables with a len(), like the + # instruction objects from instruction/ (e.g. space.clear(field.NOP())) + class FakeInstruction: + def __len__(self): + return 2 + def __call__(self, space): + return [1, 2] + + space = Space(0x100, 0x103, "test space", clear_value = FakeInstruction()) + self.assertEqual(self.rom.get_bytes(0x100, 4), [1, 2, 1, 2]) + +class TestSpaceConflicts(SpaceTestCase): + def test_overlapping_spaces_raise(self): + Space(0x100, 0x1ff, "first") + with self.assertRaises(RuntimeError): + Space(0x180, 0x280, "second") + + def test_adjacent_spaces_allowed(self): + Space(0x100, 0x1ff, "first") + Space(0x200, 0x2ff, "second") # must not raise + +class TestAllocateReserveFree(SpaceTestCase): + def test_allocate_after_free(self): + Free(0x2000, 0x2fff) + space = Allocate(Bank["C0"], 0x100, "test allocation") + self.assertEqual(space.start_address, 0x2000) + self.assertEqual(len(space), 0x100) + + def test_allocate_without_free_space_raises(self): + with self.assertRaises(RomSpaceError): + Allocate(Bank["C0"], 0x100, "test allocation") + + def test_reserve_excludes_range_from_allocation(self): + Free(0x2000, 0x20ff) + Reserve(0x2000, 0x207f, "reserved range") + space = Allocate(Bank["C0"], 0x80, "test allocation") + self.assertEqual(space.start_address, 0x2080) + + def test_write_helper_allocates_and_writes(self): + Free(0x2000, 0x2fff) + space = Write(Bank["C0"], [1, 2, 3], "test write") + self.assertEqual(len(space), 3) + self.assertEqual(self.rom.get_bytes(space.start_address, 3), [1, 2, 3]) + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_utils.py b/tests/test_utils.py new file mode 100644 index 00000000..dd69390d --- /dev/null +++ b/tests/test_utils.py @@ -0,0 +1,101 @@ +import random +import unittest + +from utils.compression import compress, decompress +from utils.flatten import flatten +from utils.intersection import intersection +from utils.shuffle_if import shuffle_if +from utils.weighted_random import weighted_random + +class TestFlatten(unittest.TestCase): + def test_scalar(self): + self.assertEqual(flatten(5), [5]) + + def test_flat_list(self): + self.assertEqual(flatten([1, 2, 3]), [1, 2, 3]) + + def test_nested_lists_and_tuples(self): + self.assertEqual(flatten([1, [2, (3, 4)], [[5]]]), [1, 2, 3, 4, 5]) + + def test_bytes_are_flattened_to_ints(self): + self.assertEqual(flatten(b"\x01\x02"), [1, 2]) + + def test_strings_are_not_flattened(self): + # strings are labels in the memory model and must stay intact + self.assertEqual(flatten(["LABEL", 1]), ["LABEL", 1]) + + def test_empty(self): + self.assertEqual(flatten([]), []) + +class TestCompression(unittest.TestCase): + def assert_round_trip(self, data): + self.assertEqual(decompress(compress(data)), data) + + def test_round_trip_repetitive_data(self): + self.assert_round_trip([1, 2, 3] * 100) + + def test_round_trip_incompressible_data(self): + rng = random.Random(42) + self.assert_round_trip([rng.randrange(256) for _ in range(500)]) + + def test_round_trip_runs(self): + self.assert_round_trip([0] * 1000) + + def test_round_trip_mixed(self): + data = list(range(256)) + [7] * 50 + list(range(0, 256, 2)) * 3 + self.assert_round_trip(data) + + def test_size_header(self): + compressed = compress([1, 2, 3] * 10) + size = int.from_bytes(bytes(compressed[:2]), "little") + self.assertEqual(size, len(compressed)) + +class TestIntersection(unittest.TestCase): + def test_intersection_preserves_first_list_order(self): + self.assertEqual(intersection([3, 1, 2, 5], [2, 3]), [3, 2]) + + def test_disjoint(self): + self.assertEqual(intersection([1, 2], [3, 4]), []) + +class TestWeightedRandom(unittest.TestCase): + def test_returns_valid_index(self): + rng_state = random.getstate() + try: + random.seed("weighted_random test") + for _ in range(100): + self.assertIn(weighted_random([1, 2, 3]), (0, 1, 2)) + finally: + random.setstate(rng_state) + + def test_zero_weights_never_chosen(self): + rng_state = random.getstate() + try: + random.seed("weighted_random test") + for _ in range(100): + self.assertEqual(weighted_random([0, 1, 0]), 1) + finally: + random.setstate(rng_state) + +class TestShuffleIf(unittest.TestCase): + def test_only_matching_elements_move(self): + rng_state = random.getstate() + try: + random.seed("shuffle_if test") + values = list(range(20)) + original = list(values) + is_even = lambda value : value % 2 == 0 + + shuffle_if(values, is_even) + + # odd elements stay in place + for index, value in enumerate(original): + if not is_even(value): + self.assertEqual(values[index], value) + # even elements are permuted amongst themselves + self.assertEqual(sorted(v for v in values if is_even(v)), + sorted(v for v in original if is_even(v))) + finally: + random.setstate(rng_state) + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_valid_rom_file.py b/tests/test_valid_rom_file.py new file mode 100644 index 00000000..0b6f9415 --- /dev/null +++ b/tests/test_valid_rom_file.py @@ -0,0 +1,25 @@ +import hashlib +import os +import tempfile +import unittest + +from valid_rom_file import get_sha256_hex, valid_rom_file + +class TestValidRomFile(unittest.TestCase): + def setUp(self): + with tempfile.NamedTemporaryFile(delete = False) as temp_file: + temp_file.write(b"not a rom" * 1000) + self.temp_path = temp_file.name + + def tearDown(self): + os.unlink(self.temp_path) + + def test_get_sha256_hex(self): + expected = hashlib.sha256(b"not a rom" * 1000).hexdigest() + self.assertEqual(get_sha256_hex(self.temp_path), expected) + + def test_invalid_rom_rejected(self): + self.assertFalse(valid_rom_file(self.temp_path)) + +if __name__ == "__main__": + unittest.main() From 9407f3d08720e82c5fc1649ca9a64d682522a94f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 11 Jun 2026 01:27:56 +0000 Subject: [PATCH 02/10] Fix inverted init_run_success setter in data/character.py The setter stored (value - MAX_RUN_SUCCESS), a negative number, while the getter computes (MAX_RUN_SUCCESS - raw). A set/get round trip returned the wrong value and the negative raw value would corrupt the bit-packed byte at init_data[21] on write. Latent bug: nothing calls the setter today, so generated seeds are unaffected; the first feature to use it would have silently corrupted character init data. Regression tests added. https://claude.ai/code/session_01QnF26DB2TYi1hpJE8L6EJr --- data/character.py | 2 +- tests/test_character.py | 43 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 tests/test_character.py diff --git a/data/character.py b/data/character.py index db012726..022cfef7 100644 --- a/data/character.py +++ b/data/character.py @@ -85,7 +85,7 @@ def init_run_success(self, value): if value < self.MIN_RUN_SUCCESS or value > self.MAX_RUN_SUCCESS: raise ValueError(f"Character.init_run_success setter: invalid value {value}") - self._init_run_success = value - self.MAX_RUN_SUCCESS + self._init_run_success = self.MAX_RUN_SUCCESS - value # initial level of characters is 3 # when new character is recruited, their level is set to the average of all other recruited characters + init_level_factor diff --git a/tests/test_character.py b/tests/test_character.py new file mode 100644 index 00000000..537cd8fc --- /dev/null +++ b/tests/test_character.py @@ -0,0 +1,43 @@ +import unittest + +from data.character import Character + +def make_character(): + init_data = [0] * 22 + name_data = [0xff] * 6 # padding bytes only, i.e. an empty name + return Character(0, init_data, name_data) + +class TestInitRunSuccess(unittest.TestCase): + # run success is stored inverted in 2 bits of init_data[21]: + # 0b11 = 2, 0b10 = 3, 0b01 = 4, 0b00 = 5 (run_value = 5 - bit_value) + def test_getter_decodes_stored_bits(self): + character = make_character() + for raw, expected in ((0b11, 2), (0b10, 3), (0b01, 4), (0b00, 5)): + character._init_run_success = raw + self.assertEqual(character.init_run_success, expected) + + def test_setter_round_trip(self): + # regression test: the setter used to store (value - MAX) instead of + # (MAX - value), corrupting the bit-packed init data byte + character = make_character() + for value in range(Character.MIN_RUN_SUCCESS, Character.MAX_RUN_SUCCESS + 1): + character.init_run_success = value + self.assertEqual(character.init_run_success, value) + self.assertIn(character._init_run_success, (0b00, 0b01, 0b10, 0b11)) + + def test_setter_rejects_out_of_range(self): + character = make_character() + with self.assertRaises(ValueError): + character.init_run_success = Character.MIN_RUN_SUCCESS - 1 + with self.assertRaises(ValueError): + character.init_run_success = Character.MAX_RUN_SUCCESS + 1 + +class TestInitLevelFactor(unittest.TestCase): + def test_round_trip(self): + character = make_character() + for adjustment in (0, 2, 5, -3): + character.init_level_factor = adjustment + self.assertEqual(character.init_level_factor, adjustment) + +if __name__ == "__main__": + unittest.main() From 0a5e176372eac86e500914a003f672be10258797 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 11 Jun 2026 01:28:49 +0000 Subject: [PATCH 03/10] Fix NameError in objective quest-lookup failure path The assertion message referenced bare suplex_train_quest_name instead of cls.suplex_train_quest_name, so a failed lookup raised NameError instead of the intended diagnostic. Use an explicit RuntimeError so the check also survives python -O. https://claude.ai/code/session_01QnF26DB2TYi1hpJE8L6EJr --- objectives/objective.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/objectives/objective.py b/objectives/objective.py index 323429d9..2a326c13 100644 --- a/objectives/objective.py +++ b/objectives/objective.py @@ -128,5 +128,5 @@ def _init_suplex_train_quest_value(cls): if value != 'r' and quest_bit[value].name == cls.suplex_train_quest_name: cls.suplex_train_quest_value = value return - assert False, f"'{suplex_train_quest_name}' quest value not found" + raise RuntimeError(f"'{cls.suplex_train_quest_name}' quest value not found") Objective._init_suplex_train_quest_value() From 0f5cf0c9316ab09a7b148567763c6c35cb2971cf Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 11 Jun 2026 01:28:49 +0000 Subject: [PATCH 04/10] Remove dead code in args/challenges.py Delete an unreachable 'return opts' (opts was never defined) after the options() return statement, and the first of two definitions of _format_spells_log_entries, which was silently shadowed by the second. No behavior change: the surviving definition is the one that already ran. https://claude.ai/code/session_01QnF26DB2TYi1hpJE8L6EJr --- args/challenges.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/args/challenges.py b/args/challenges.py index 046e7c71..34e12f41 100644 --- a/args/challenges.py +++ b/args/challenges.py @@ -115,14 +115,6 @@ def options(args): ("Remove Learnable Spells", args.remove_learnable_spell_ids, "remove_learnable_spell_ids"), ("No Saves", args.no_saves, "no_saves"), ] - - return opts -def _format_spells_log_entries(spell_ids): - from constants.spells import id_spell - spell_entries = [] - for i, spell_id in enumerate(spell_ids): - spell_entries.append(("", id_spell[spell_id], f"rls_{i}")) - return spell_entries def _format_spells_log_entries(spell_ids): from constants.spells import id_spell From 6712481cc48960087fbe3f55cf80cbb673986c44 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 11 Jun 2026 01:28:49 +0000 Subject: [PATCH 05/10] Remove dead RewardType.NONE check in whelk event Reward.type is initialized to Python None and RewardType.NONE is never assigned anywhere, so the early-return could never trigger. By the time mod() runs, all rewards have been assigned a real type. https://claude.ai/code/session_01QnF26DB2TYi1hpJE8L6EJr --- event/whelk.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/event/whelk.py b/event/whelk.py index 91966d70..47efc50b 100644 --- a/event/whelk.py +++ b/event/whelk.py @@ -16,9 +16,6 @@ def init_event_bits(self, space): ) def mod(self): - if self.reward.type == RewardType.NONE: - return - self.dialog_mod() self.entrance_event_mod() self.cleanup_mod() From c8b1d3822e835ac2f3cb5a5e76461444fc4ba25e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 11 Jun 2026 01:29:46 +0000 Subject: [PATCH 06/10] Fix silent failures: compression truncation, metadata group condition, scaling description - utils/compression.py: compress() printed an error and returned data with a truncated size header when output exceeded 65535 bytes, which would silently corrupt whatever consumed it; raise ValueError instead (regression test added) - metadata/flag_metadata_writer.py: 'if type(group_title):' was always true (a type object is truthy); replace with an explicit str/None/callable dispatch that preserves the existing manifest output exactly - battle/scaling.py: the xp/gp scaling routine was labeled 'scale hp/mp' in its space description (debug/error output only, not ROM content) https://claude.ai/code/session_01QnF26DB2TYi1hpJE8L6EJr --- battle/scaling.py | 2 +- metadata/flag_metadata_writer.py | 7 +++++-- tests/test_utils.py | 8 ++++++++ utils/compression.py | 4 ++-- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/battle/scaling.py b/battle/scaling.py index 27b2e502..592242b4 100644 --- a/battle/scaling.py +++ b/battle/scaling.py @@ -185,7 +185,7 @@ def scale_xp_gp_mod(self): asm.STA(0xea, asm.DIR), asm.JMP(self.scale_value, asm.ABS), ] - space = Write(Bank.C2, src, "scale hp/mp") + space = Write(Bank.C2, src, "scale xp/gp") self.scale_xp_gp = space.start_address if args.xp_gp_scaling: diff --git a/metadata/flag_metadata_writer.py b/metadata/flag_metadata_writer.py index c8337299..ab1712c9 100644 --- a/metadata/flag_metadata_writer.py +++ b/metadata/flag_metadata_writer.py @@ -57,8 +57,11 @@ def get_flag_metadata(self): self.metadata[key].args = action.metavar if action.choices is not None and isinstance(action.choices, list) and not isinstance(action.choices, range): self.metadata[key].allowed_values = list(action.choices) - if type(group_title): - self.metadata[key].group = group_title if type(group_title) == str else None if group_title == None else group_title() + # group titles may be a plain string, None, or a callable returning the name + if isinstance(group_title, str) or group_title is None: + self.metadata[key].group = group_title + else: + self.metadata[key].group = group_title() if getattr(action, 'mutually_exclusive_group_title', None) is not None: self.metadata[key].mutually_exclusive_group = action.mutually_exclusive_group_title if getattr(action, 'choices', None) is not None: diff --git a/tests/test_utils.py b/tests/test_utils.py index dd69390d..b134a817 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -50,6 +50,14 @@ def test_size_header(self): size = int.from_bytes(bytes(compressed[:2]), "little") self.assertEqual(size, len(compressed)) + def test_oversized_output_raises(self): + # incompressible data grows ~9/8x when compressed; this used to + # print an error and return a truncated size header instead of raising + rng = random.Random(42) + data = [rng.randrange(256) for _ in range(60000)] + with self.assertRaises(ValueError): + compress(data) + class TestIntersection(unittest.TestCase): def test_intersection_preserves_first_list_order(self): self.assertEqual(intersection([3, 1, 2, 5], [2, 3]), [3, 2]) diff --git a/utils/compression.py b/utils/compression.py index ab9afb1d..0705fbde 100644 --- a/utils/compression.py +++ b/utils/compression.py @@ -73,8 +73,8 @@ def compress(data): size = len(result) + 2 if size > MAX_COMPRESS_SIZE: - print(f"Error: compress: data too large (compressed size {size} > 65535)") - size = MAX_COMPRESS_SIZE + # a wrong size header would silently corrupt the decompressed data + raise ValueError(f"compress: data too large (compressed size {size} > {MAX_COMPRESS_SIZE})") return list(size.to_bytes(2, "little")) + result def decompress(data): From 9e69bc4bc80dd2d6a97539cb77bbe99195fd80ff Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 11 Jun 2026 01:36:55 +0000 Subject: [PATCH 07/10] Replace validation asserts with informative exceptions in seed-generation paths Asserts are stripped under python -O and give users a bare AssertionError when they fire. Converted to explicit checks raising RuntimeError/ValueError with context, and to parser.error() for user input validation: - event/event_reward.py: the documented gating-deadlock failure in choose_reward() now reports which reward types were requested - event/events.py: the CHARACTER_ESPER_ONLY_REWARDS count validation - memory/space.py: clear() size mismatch now reports both sizes - instruction/field/instructions.py: Set/ClearEventBit range checks (also reject negative bits, which would corrupt the opcode) - args/starting_party.py: duplicate/too-many start characters now produce a clean CLI error like other arg modules instead of AssertionError No control-flow change when checks pass; seed output unchanged. https://claude.ai/code/session_01QnF26DB2TYi1hpJE8L6EJr --- args/starting_party.py | 6 ++++-- event/event_reward.py | 4 +++- event/events.py | 3 ++- instruction/field/instructions.py | 6 ++++-- memory/space.py | 3 ++- 5 files changed, 15 insertions(+), 7 deletions(-) diff --git a/args/starting_party.py b/args/starting_party.py index 6965c781..cfb20dbf 100644 --- a/args/starting_party.py +++ b/args/starting_party.py @@ -35,10 +35,12 @@ def process(args): args.start_chars = ["random"] else: # ensure only 4 starting characters and no duplicates (except random) - assert len(args.start_chars) <= 4 + if len(args.start_chars) > 4: + args.parser.error("starting-party: at most 4 starting characters allowed") start_chars_found = set() for char in args.start_chars: - assert (char == "random" or char == "randomngu" or char not in start_chars_found) + if char not in ("random", "randomngu") and char in start_chars_found: + args.parser.error(f"starting-party: duplicate starting character '{char}'") start_chars_found.add(char) def flags(args): diff --git a/event/event_reward.py b/event/event_reward.py index b8a8ed3b..a1d43945 100644 --- a/event/event_reward.py +++ b/event/event_reward.py @@ -49,7 +49,9 @@ def choose_reward(possible_types, characters, espers, items): # tried all possible_rewards and none were available # probably running out of chars and espers and need to make item rewards possible for more events - assert(item_possible) + if not item_possible: + raise RuntimeError(f"choose_reward: no rewards available for types {possible_types}: " + "characters/espers are exhausted and this slot does not allow items") return (items.get_good_random(), RewardType.ITEM) # Documentation from AtmaTek: diff --git a/event/events.py b/event/events.py index c5c072a7..82fffd20 100644 --- a/event/events.py +++ b/event/events.py @@ -171,4 +171,5 @@ def validate(self, events): for event in events: char_esper_checks += [r for r in event.rewards if r.possible_types == (RewardType.CHARACTER | RewardType.ESPER)] - assert len(char_esper_checks) == CHARACTER_ESPER_ONLY_REWARDS, f"Number of char/esper only checks changed - Check usages of CHARACTER_ESPER_ONLY_REWARDS and ensure no breaking changes. Expected: {CHARACTER_ESPER_ONLY_REWARDS}, Actual: {len(char_esper_checks)}" + if len(char_esper_checks) != CHARACTER_ESPER_ONLY_REWARDS: + raise RuntimeError(f"Number of char/esper only checks changed - Check usages of CHARACTER_ESPER_ONLY_REWARDS and ensure no breaking changes. Expected: {CHARACTER_ESPER_ONLY_REWARDS}, Actual: {len(char_esper_checks)}") diff --git a/instruction/field/instructions.py b/instruction/field/instructions.py index 0b8f2560..a8b33be2 100644 --- a/instruction/field/instructions.py +++ b/instruction/field/instructions.py @@ -626,7 +626,8 @@ def __str__(self): class SetEventBit(_Instruction): def __init__(self, event_bit): self.event_bit = event_bit - assert self.event_bit <= 0x6ff + if not 0 <= self.event_bit <= 0x6ff: + raise ValueError(f"SetEventBit: invalid event bit {hex(self.event_bit)}, must be within 0x000-0x6ff") opcode = 0xd0 + (self.event_bit // 0x100) * 2 arg = self.event_bit & 0xff @@ -638,7 +639,8 @@ def __str__(self): class ClearEventBit(_Instruction): def __init__(self, event_bit): self.event_bit = event_bit - assert self.event_bit <= 0x6ff + if not 0 <= self.event_bit <= 0x6ff: + raise ValueError(f"ClearEventBit: invalid event bit {hex(self.event_bit)}, must be within 0x000-0x6ff") opcode = 0xd1 + (self.event_bit // 0x100) * 2 arg = self.event_bit & 0xff diff --git a/memory/space.py b/memory/space.py index 39eb51eb..9cad3bb1 100644 --- a/memory/space.py +++ b/memory/space.py @@ -98,7 +98,8 @@ def clear(self, value): values = [value] * len(self) values = self._invoke_callables(values) - assert len(self) == len(values) # do values evenly fill space? + if len(self) != len(values): # do values evenly fill space? + raise ValueError(f"clear: {len(values)} values do not evenly fill space {str(self)} ({len(self)} bytes)") Space.rom.set_bytes(self.start_address, values) self._next_address = self.start_address From 755682196c6ad4086b4a13877bfaf620574cc6cc Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 11 Jun 2026 01:38:09 +0000 Subject: [PATCH 08/10] Guard unbounded retry loops so deadlocks fail loudly instead of hanging Three rejection-sampling loops could spin forever if their progress assumption broke (all remaining candidates already hold the next item/spell, or no gated slot is unlockable): - data/espers.py shuffle_spells and data/shops.py shuffle: raise after 10000 consecutive failed picks or when the candidate pool empties - event/events.py character_gating_mod: raise a descriptive error when no unfilled character slot is unlocked (previously crashed with TypeError) - utils/truncated_discrete_distribution.py: convert unbounded recursion to a capped loop; impossible bounds now raise ValueError (test added) Guards consume no RNG and the success paths are unchanged, so seed output is identical. https://claude.ai/code/session_01QnF26DB2TYi1hpJE8L6EJr --- data/espers.py | 12 ++++++++++++ data/shops.py | 12 ++++++++++++ event/events.py | 4 ++++ tests/test_utils.py | 19 +++++++++++++++++++ utils/truncated_discrete_distribution.py | 22 ++++++++++++++++------ 5 files changed, 63 insertions(+), 6 deletions(-) diff --git a/data/espers.py b/data/espers.py index bf2c0957..02047b60 100644 --- a/data/espers.py +++ b/data/espers.py @@ -101,10 +101,20 @@ def shuffle_spells(self): random.shuffle(spell_counts) + # if every esper with open slots already knows the next spell, the loop + # below can no longer make progress; fail loudly instead of hanging. + # the guards consume no rng so seed output is unchanged + stalled_picks = 0 + MAX_STALLED_PICKS = 10000 + while len(spells) > 0: + if not esper_indices or stalled_picks > MAX_STALLED_PICKS: + raise RuntimeError(f"shuffle_spells: cannot place {len(spells)} remaining spells, " + f"{len(esper_indices)} espers have open slots") esper_index = random.choice(esper_indices) esper = self.espers[esper_index] if not esper.has_spell(spells[-1].id): + stalled_picks = 0 spell = spells.pop() if self.args.esper_spells_shuffle_random_rates: esper.add_spell(spell.id, random.choice(Esper.LEARN_RATES)) @@ -112,6 +122,8 @@ def shuffle_spells(self): esper.add_spell(spell.id, spell.rate) if esper.spell_count == spell_counts[esper_index]: esper_indices.remove(esper_index) + else: + stalled_picks += 1 def randomize_spells(self): for esper in self.espers: diff --git a/data/shops.py b/data/shops.py index 70f1c4c3..c96e0082 100644 --- a/data/shops.py +++ b/data/shops.py @@ -69,14 +69,26 @@ def shuffle(self): random.shuffle(item_counts) + # if every shop with open slots already stocks the next item, the loop + # below can no longer make progress; fail loudly instead of hanging. + # the guards consume no rng so seed output is unchanged + stalled_picks = 0 + MAX_STALLED_PICKS = 10000 + while len(items) > 0: + if not shop_indices or stalled_picks > MAX_STALLED_PICKS: + raise RuntimeError(f"shops shuffle: cannot place {len(items)} remaining items, " + f"{len(shop_indices)} shops have open slots") shop_index = random.choice(shop_indices) shop = type_shops[shop_type][shop_index] if not shop.contains(items[-1]): + stalled_picks = 0 item = items.pop() shop.append(item) if shop.item_count == item_counts[shop_index]: shop_indices.remove(shop_index) + else: + stalled_picks += 1 def random_tiered(self): def get_item(item_type, exclude = None): diff --git a/event/events.py b/event/events.py index 82fffd20..9017838d 100644 --- a/event/events.py +++ b/event/events.py @@ -126,6 +126,10 @@ def character_gating_mod(self, events, name_event): unlocked_slot_iterations.append(slot_iterations[slot]) # pick slot for the next character weighted by number of iterations each slot has been available + if not unlocked_slots: + raise RuntimeError("character gating deadlock: " + f"{self.characters.get_available_count()} characters left to assign " + "but no unfilled character slots are currently unlocked") slot_index = weighted_reward_choice(unlocked_slot_iterations, iteration) slot = unlocked_slots[slot_index] slot.id = self.characters.get_random_available() diff --git a/tests/test_utils.py b/tests/test_utils.py index b134a817..3e636989 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -5,6 +5,7 @@ from utils.flatten import flatten from utils.intersection import intersection from utils.shuffle_if import shuffle_if +from utils.truncated_discrete_distribution import truncated_discrete_distribution from utils.weighted_random import weighted_random class TestFlatten(unittest.TestCase): @@ -84,6 +85,24 @@ def test_zero_weights_never_chosen(self): finally: random.setstate(rng_state) +class TestTruncatedDiscreteDistribution(unittest.TestCase): + def setUp(self): + self._rng_state = random.getstate() + random.seed("truncated_discrete_distribution test") + + def tearDown(self): + random.setstate(self._rng_state) + + def test_respects_bounds(self): + for _ in range(200): + result = truncated_discrete_distribution(10, 5, 8, 12) + self.assertTrue(8 <= result <= 12) + + def test_impossible_bounds_raise(self): + # used to recurse forever; e.g. minimum above maximum + with self.assertRaises(ValueError): + truncated_discrete_distribution(10, 1, minimum = 110, maximum = 105) + class TestShuffleIf(unittest.TestCase): def test_only_matching_elements_move(self): rng_state = random.getstate() diff --git a/utils/truncated_discrete_distribution.py b/utils/truncated_discrete_distribution.py index 921d9178..6403cc1a 100644 --- a/utils/truncated_discrete_distribution.py +++ b/utils/truncated_discrete_distribution.py @@ -1,9 +1,19 @@ # not a "real" distribution, the discretization and clamping skew it def truncated_discrete_distribution(mean, stddev, minimum = None, maximum = None): import random - result = round(random.gauss(mean, stddev)) - if minimum and result < minimum: - return truncated_discrete_distribution(mean, stddev, minimum, maximum) - if maximum and result > maximum: - return truncated_discrete_distribution(mean, stddev, minimum, maximum) - return result + + # rejection sampling: retry until a value lands within the bounds. + # the attempt cap turns impossible/near-impossible bounds into an error + # instead of an infinite loop; each attempt consumes exactly one + # random.gauss call, the same as the previous recursive implementation + MAX_ATTEMPTS = 10000 + for _ in range(MAX_ATTEMPTS): + result = round(random.gauss(mean, stddev)) + if minimum and result < minimum: + continue + if maximum and result > maximum: + continue + return result + raise ValueError(f"truncated_discrete_distribution: no value within " + f"[{minimum}, {maximum}] after {MAX_ATTEMPTS} attempts " + f"(mean {mean}, stddev {stddev})") From 72473eb47b344b110b37d8333ddc88d5ea432762 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 11 Jun 2026 01:38:55 +0000 Subject: [PATCH 09/10] Replace bare except clauses in args modules with specific exceptions - starting_gold_items.py: int() conversion failures are ValueError - misc_magic/espers/scaling/lores/items menu(): .replace() on a non-string entry value raises AttributeError; catching everything could hide real bugs in menu generation (misc_magic.py has CRLF line endings, preserved as-is.) https://claude.ai/code/session_01QnF26DB2TYi1hpJE8L6EJr --- args/espers.py | 4 ++-- args/items.py | 4 ++-- args/lores.py | 4 ++-- args/misc_magic.py | 4 ++-- args/scaling.py | 4 ++-- args/starting_gold_items.py | 6 +++--- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/args/espers.py b/args/espers.py index eb10d0d7..a93c81b0 100644 --- a/args/espers.py +++ b/args/espers.py @@ -204,8 +204,8 @@ def menu(args): if key == "Equipable": value = value.replace("Random", "") entries[index] = (key, value, unique_name) - except: - pass + except AttributeError: + pass # value is not a string (e.g. a submenu), leave entry unchanged return (name(), entries) def log(args): diff --git a/args/items.py b/args/items.py index fb0bc0c1..f85632ad 100644 --- a/args/items.py +++ b/args/items.py @@ -247,8 +247,8 @@ def menu(args): value = value.replace("Original + Random", "Original + ") value = value.replace("Shuffle + Random", "Shuffle + ") entries[index] = (key, value, unique_name) - except: - pass + except AttributeError: + pass # value is not a string (e.g. a submenu), leave entry unchanged return (name(), entries) diff --git a/args/lores.py b/args/lores.py index 0b3ef227..98d93672 100644 --- a/args/lores.py +++ b/args/lores.py @@ -86,8 +86,8 @@ def menu(args): value = value.replace("Random Value ", "") value = value.replace("Random Percent ", "") entries[index] = (key, value, unique_name) - except: - pass + except AttributeError: + pass # value is not a string (e.g. a submenu), leave entry unchanged return (name(), entries) def log(args): diff --git a/args/misc_magic.py b/args/misc_magic.py index c886c558..637737b4 100644 --- a/args/misc_magic.py +++ b/args/misc_magic.py @@ -53,8 +53,8 @@ def menu(args): value = value.replace("Random Value ", "") value = value.replace("Random Percent ", "") entries[index] = (key, value, unique_name) - except: - pass + except AttributeError: + pass # value is not a string (e.g. a submenu), leave entry unchanged return (name(), entries) def log(args): diff --git a/args/scaling.py b/args/scaling.py index ed64f458..95ff0197 100644 --- a/args/scaling.py +++ b/args/scaling.py @@ -310,8 +310,8 @@ def menu(args): value = value.replace("Characters + Espers", "C + E") value = value.replace("Bosses + Dragons", "B + D") entries[index] = (key, value, unique_name) - except: - pass + except AttributeError: + pass # value is not a string (e.g. a submenu), leave entry unchanged return (name(), entries) def log(args): diff --git a/args/starting_gold_items.py b/args/starting_gold_items.py index e5ad3291..3d833055 100644 --- a/args/starting_gold_items.py +++ b/args/starting_gold_items.py @@ -55,7 +55,7 @@ def __init__(self, _nid, min, max): item_id = 0 try: item_id = int(values[index]) - except: + except ValueError: args.parser.error(f"start-items: Failed to convert value into an int '{values[index]}'") if item_id < 0 or item_id >= 255: args.parser.error(f"start-items: '{item_id}' is an invalid value for an item id. It must be between 0-254") @@ -63,7 +63,7 @@ def __init__(self, _nid, min, max): min = 0 try: min = int(values[index + 1]) - except: + except ValueError: args.parser.error(f"start-items: Failed to convert value into an int '{values[index+1]}'") if min < 0 or min > 99: args.parser.error(f"start-items: '{min}' is an invalid min for an item. It must be between 0 and 99") @@ -71,7 +71,7 @@ def __init__(self, _nid, min, max): max = 0 try: max = int(values[index + 2]) - except: + except ValueError: args.parser.error(f"start-items: Failed to convert value into an int '{values[index+2]}'") if max <= 0 or max > 99: args.parser.error(f"start-items: '{max}' is an invalid count for an item. It must be between 1-99") From 471fbd8bdbccdc648501cbcd6aa5d039d0525747 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 11 Jun 2026 01:39:30 +0000 Subject: [PATCH 10/10] Add Pillow import guard to PNG tools; write spoiler log as UTF-8 - graphics/tools/png_{portrait,sprite}.py import PIL, contradicting the documented 'standard library only' requirement; a missing install now produces a clear message instead of a bare ImportError. agents.md updated to document the optional dependency. - log/__init__.py: spoiler log encoding was platform-dependent (cp1252 on Windows); now explicitly utf-8. https://claude.ai/code/session_01QnF26DB2TYi1hpJE8L6EJr --- agents.md | 2 ++ graphics/tools/png_portrait.py | 6 +++++- graphics/tools/png_sprite.py | 6 +++++- log/__init__.py | 3 ++- 4 files changed, 14 insertions(+), 3 deletions(-) diff --git a/agents.md b/agents.md index 6d60035b..ea936dd2 100644 --- a/agents.md +++ b/agents.md @@ -12,6 +12,8 @@ This document is written for autonomous AI coding agents (such as Antigravity, S ### 1.1 Python Dependencies The randomizer runs entirely on Python 3 with the standard library. No external pip installations are required. +Exception: the optional developer tools `graphics/tools/png_portrait.py` and `graphics/tools/png_sprite.py` (PNG-to-sprite conversion) require Pillow (`pip install Pillow`). The randomizer itself never imports it. + ### 1.2 Target ROM File To verify ROM modifications and successfully run a seed generation test, a valid Super Nintendo FF6 ROM file is required: - **Filename**: ff3.smc (located in the workspace root). diff --git a/graphics/tools/png_portrait.py b/graphics/tools/png_portrait.py index 288b79f7..4abb8ca1 100644 --- a/graphics/tools/png_portrait.py +++ b/graphics/tools/png_portrait.py @@ -57,7 +57,11 @@ def write_sprite(output_prefix, sprite, tile_indices): output.write(bytes(sprite.data)) def convert(image_path): - from PIL import Image + try: + from PIL import Image + except ImportError: + raise ImportError("this developer tool requires the Pillow library (pip install Pillow); " + "the randomizer itself does not need it") from None image = Image.open(image_path) import os diff --git a/graphics/tools/png_sprite.py b/graphics/tools/png_sprite.py index 5c807b6a..e14ab1fb 100644 --- a/graphics/tools/png_sprite.py +++ b/graphics/tools/png_sprite.py @@ -98,7 +98,11 @@ def write_sprite(output_prefix, sprite, tile_indices): output.write(bytes(sprite.data)) def convert(image_path): - from PIL import Image + try: + from PIL import Image + except ImportError: + raise ImportError("this developer tool requires the Pillow library (pip install Pillow); " + "the randomizer itself does not need it") from None image = Image.open(image_path) import os diff --git a/log/__init__.py b/log/__init__.py index d8431cd0..3c72034a 100644 --- a/log/__init__.py +++ b/log/__init__.py @@ -8,7 +8,8 @@ import sys logging.basicConfig(stream = sys.stdout, filemode = 'w', level = logging.INFO, format = "%(message)s") else: - logging.basicConfig(filename = log_file, filemode = 'w', level = logging.INFO, format = "%(message)s") + # explicit utf-8 so log content is identical across platforms (notably Windows) + logging.basicConfig(filename = log_file, filemode = 'w', level = logging.INFO, format = "%(message)s", encoding = "utf-8") hash = ', '.join([entry.name for entry in args.sprite_hash]) import time