fix(placement-plan): check the exit inventory and refuse failed placements by zoeyrose · Pull Request #47 · atrinik/content-toolkit

zoeyrose · 2026-10-18T17:19:58Z

Summary

Closes #46. This fixes the three placement-plan verifier and generator gaps deferred from #45's final review. The verifier now checks the exit records against a complete source exit inventory, and each stairwell's replaces entries against their exit records. A group whose placement fails stays unplaced and blocks publication. The committed plan is unchanged apart from the new inventory and bookkeeping fields: the same placements, stairwells and exits, and the same PNGs.

Implementation / behavior

1. Complete crossing-exit inventory (#45 thread r4306554103)

build_plan.py publishes exit_inventory: every exit object read from the content (1,200), keyed by source_map and line, with its legacy form and resolved target. The plan's schema becomes atrinik-overworld-placement-plan/4.
verify_plan.py derives the expected record copies from the inventory: one copy in each group a crossing exit touches, and overworld-to-overworld exits in the overworld's internal_exits. It requires exactly those copies (no missing, extra or misfiled copies), each equal to its inventory entry. So deleting either copy of mine_entrance_a:641, or both, now fails.
It re-derives the summary crossing counts, the per-group crossing_exits_after_placement, the exits into separate planes and the portal reasons. The inventory size must equal summary.exit_objects.
An exit record must carry plane_source / plane_target exactly when both of its ends are placed. Before, deleting them silently skipped the endpoint check. overworld_plane_source / _target are now checked too.
Every other copy of an exit is compared with its record or inventory entry: blocked stairwell sites (column, sloped, relayering), relayering.exits, wired and kept dangling exits, data_problems.exits and each placed group's main_connection.
Generator fix found along the way: an overworld exit wired to another overworld map was appended to the crossing lists, which the overworld group never writes out, so no copy of it was published. It now goes into internal_exits. No current exit is affected.

2. Stairwell replaces vs. the exit record (#45 thread r4306554106)

Each replaces entry must equal its indexed record in source tile, target map and tile, archetype, and plane source and target. Its goes must match the record's direction.

3. Failed groups stay unplaced (#45 thread r4306554110)

The pool loop is now place_pool(). A group whose place_group() fails is kept out of placement_order, is never retried, and is listed in unplaced_place_groups / unplaced_uncertain_groups and the new summary.failed_placements. Groups reachable only through it stay unplaced as well. A failed tentative alignment (rockforge_c) is recorded the same way.
The verifier rejects any status: failed placement, and any place group without a proposed placement or uncertain group without a tentative one. It also re-derives the placement order, the unplaced lists and failed_placements from the records. The build's verify-before-publish therefore refuses such a plan.
The radius-10 sensitivity run now lists any groups it could not place (groups_unplaced; none today).

Two checks. verify_plan.py re-derives the placement rules from the JSON alone; tests and CI run that. With --content DIR it also rebuilds the plan from the content at content_git_head with the generator, and requires the published JSON to equal the rebuild as canonical JSON text, with only content_root (the checkout path) aside. That fidelity check catches any edit that keeps the JSON consistent with itself. build_plan.py --check likewise compares every artifact byte for byte.

Review rounds extended the same idea to the rest of the exit and placement data. The records and the inventory share exit_source_fields(), and every exit record field is either equal to the inventory or re-derived: placement fields present only when both ends are placed, distances, labels, portal reason and text, slope_reachable. Unknown fields are rejected. Exit data problems are re-derived entry by entry. Stairwell shape, ids, owners, max_bend and intermediate decoration are re-derived. Blocked sites must list qualifying exits between their own owners. Group classifications and summary counts are checked. Each placement's offsets must follow from its ideal spot, and the ideal from its main connection. The verifier's docstring lists what it deliberately does not re-derive: the generator's explanations and evidence.

Generation is still deterministic, and the staging and verify-before-publish steps are unchanged.

Validation

Regenerated from the separate clone at content 3ad20150da76, clean under arch/ and maps/. At the base commit, a rerun reproduced all 34 committed artifacts byte-for-byte. With this change, two runs, one in place and one into a scratch directory, are byte-identical across all 34 artifacts. Only placement-plan.json and REPORT.md change; the PNGs are identical.
python3 -I scripts/verify_plan.py placement-plan.json: ok: true, with 1,007 record copies matched against 1,200 inventory exits.
verify_plan.py placement-plan.json --content <checkout>: ok, from the input clone and from a fresh clone at another path.
python3 -I scripts/test_placement_plan.py: 84 tests pass in about 40 s serially or 10 s with 8 workers (3 of them only when PLACEMENT_PLAN_CONTENT names a content checkout); tools/validate.sh runs the suite. Each tampers with a copy of the committed plan, or drives place_pool() with a failing group. Run against the old verify_plan.py, every tampering case is accepted or crashes the verifier instead of failing cleanly.
Cross-check: a plan built with view radius 10 (three groups placed differently) verifies with no errors. That shows the verifier's re-derivations match the generator outside the committed configuration.
End to end, with MAX_SHIFT = MAX_EXTRA_LAYERS = 0 to force failures into a scratch output directory holding sentinel files: 8 groups fail and 9 are unplaced (rockforge_b is reachable only through a failed group). The only verification errors are those failures, the build prints verification FAILED; nothing written, and the sentinels are untouched.
build_plan.py --check: all 34 committed artifacts are byte-identical to a fresh run. A tampered copy that verify_plan.py accepts (an internal exit dropped with its count) fails --check.
tools/validate.sh: passes (exit 0), with the CI-pinned syft 1.50.0 (SHA-256 verified) on PATH. git diff --check: clean. actionlint is not installed locally, and no workflow files changed.

Limitations / follow-up

Explanations and evidence (classification evidence, orientation votes, blocked-site and wiring evidence, relayering prose, the v1 comparison and the sensitivity run) are not re-derived; the verifier's docstring lists them.
The fidelity check (--content) and --check need a content checkout at content_git_head, so CI runs only the JSON-only check. The ContentFidelity tests skip there.
Which dangling exits get wired is not re-derived. The wired set is cross-checked across data_problems.dangling_exits_wired, summary.wired_exits and the records, but recomputing the wiring rule would duplicate the generator.
tools/validate.sh now runs test_placement_plan.py (about 10-15 s in parallel workers), the one change outside migration/placement-plan/.

🤖 Generated with Claude Code

…ments Close the verifier and generator gaps deferred from #45 (issue #46). - build_plan.py publishes exit_inventory, every exit object read from the content. verify_plan.py derives the expected exit record copies from it (one per group a crossing exit touches, the overworld's own exits in internal_exits) and requires exactly those copies, each equal to its inventory entry. It re-derives the summary and per-group crossing counts, requires plane endpoints on every exit whose ends are placed, and compares every other copy of an exit (blocked sites, relayering exits, wired and kept dangling exits, data problems, main connections) with its record or inventory entry. - Each stairwell replaces entry must equal its exit record: source tile, target map and tile, archetype, plane endpoints and direction. - A group whose placement fails stays unplaced: it leaves the placement order, is listed as unplaced and in the new summary.failed_placements, and is never retried. The verifier rejects a failed placement and any place or uncertain group without its placement, so the build does not publish such a plan. The view-radius sensitivity run lists its unplaced groups. - An overworld exit wired to another overworld map goes into the overworld's internal_exits; before, no list held a copy of it. - test_placement_plan.py: tampered copies of the committed plan for each check, and place_pool() with a failing group. No content checkout needed. Regenerated from content 3ad20150da76: placement-plan.json gains exit_inventory and summary.failed_placements, REPORT.md's check line and Reproducing section change, and the PNGs are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

github-actions · 2026-10-18T17:20:09Z

Dependency Review

✅ No vulnerabilities or license issues found.

Scanned Files

None

zoeyrose · 2026-10-18T17:20:14Z

@codex review Please do a deep, thorough review of the whole diff and the code it touches and report every issue you find in this pass, including P2s, not only the most important few.

chatgpt-codex-connector · 2026-10-18T17:20:33Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-19T01:27:47.771976Z	`7564152`	Manual request
🔒 Security Review	✅ Completed 2026-10-19T01:44:59.931840Z	`7564152`	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f7ad8debee

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Address the first Codex review round on #47. - Exit records and the exit inventory now share exit_source_fields(), and every content field of a record (tiles, target, archetype, form, direction, flags) must equal its inventory entry. A record may hold no field the verifier does not know. Wired exits no longer carry an authored_target of nulls. - A record has all of its placement fields exactly when both of its ends are placed, and none of them otherwise, so a partial endpoint pair is rejected. xy_distance and layer_delta must follow from the plane ends. The stairwell label must match becomes, and a portal's reason and slope_reachable are re-derived the way the generator decides them. - The exit data problems (kind and severity per exit) are re-derived from the inventory, which now also carries the reader's target problem and an explicit last_heal 0. - A blocked stairwell site may list only exits that need a stairwell, so the existing end check covers every listed exit. - A tentative placement needs an uncertain group, or an unreachable group with an alignment hint. - The alignment-hint retry runs only for groups no pool attempted, so a failed group cannot come back as both placed and failed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

zoeyrose · 2026-10-18T17:44:33Z

Round 1 (Codex review of f7ad8de): all six findings fixed in e36e51e.

Partial endpoint pairs on unplaced exits (r4307985224): placement fields are checked field by field. All five are required when both ends are placed, and none (nor any label field) otherwise. xy_distance and layer_delta are re-derived as well.
Tentative placements (r4307985228): allowed only on uncertain groups, or on separate / unreachable groups with an alignment-hint tentative_reason. Rockforge C is committed as separate/unreachable by design, so the stricter rule keeps it.
Forced retry after a pool failure (r4307985230): the TENTATIVE_ALIGN attempt now skips groups any pool already attempted, so a group can't be both failed and placed.
Unresolved-exit data problems (r4307985233): each exit's data-problem kinds and severities are re-derived from the inventory, which now carries the target problem and an explicit last_heal 0.
Unrelated exits at blocked sites (r4307985236): every exit a blocked site lists must need a stairwell, so the end check covers all of them.
Exit direction (r4307985239): the records and the inventory share exit_source_fields(). Every content field of a record, vertical included, must equal its inventory entry, and unknown record fields are rejected. Portal reasons and slope_reachable are now re-derived the same way the generator decides them.

Validation for e36e51e:

test_placement_plan.py: 37 tests pass.
verify_plan.py: ok.
Regeneration from content 3ad20150da76: byte-identical across a scratch run and the in-place run. The plan changes only in the richer exit_inventory and in the 5 wired records losing their all-null authored_target; the PNGs and REPORT.md are unchanged.
Extra builds with view radius 10, and with shifts and extra layers disabled (8 groups fail), verify with no errors other than the expected placement failures.

zoeyrose · 2026-10-18T17:44:35Z

@codex review Please do a deep, thorough review of the whole diff and the code it touches and report every issue you find in this pass, including P2s, not only the most important few.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e36e51eaa5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Address the second Codex review round on #47. A placement must name a main connection unless an alignment hint forced it, and never both. The connection's kind must be "in" or "out", it must be one of the group's exit records of that kind, and its other end must be the overworld or a group placed earlier. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

zoeyrose · 2026-10-18T18:01:20Z

Round 2 (Codex review of e36e51e): the one finding is fixed in 808ea4a.

Main connections for ordinary placements (r4308077232): every placement has a main_connection unless an alignment hint forced it, and never both. Its kind is checked (in / out). It must be one of the group's exit records of that kind, to the overworld or to a group placed earlier.

Validation for 808ea4a: test_placement_plan.py passes 38 tests and verify_plan.py is ok. The plan is unchanged, since only the verifier and tests changed. A radius-10 build still verifies with no errors.

zoeyrose · 2026-10-18T18:01:23Z

@codex review Please do a deep, thorough review of the whole diff and the code it touches and report every issue you find in this pass, including P2s, not only the most important few.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 808ea4acfb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Address the third Codex review round on #47. - Every inventory entry's source map must belong to a group, including exits without a destination. - The inventory carries the remaining facts the exit data problems come from: the tile-link slot of a tiled exit without a link, a dynamic exit's authored path, and a target problem with its authored target. The verifier re-derives every data-problem entry field by field, details and notes included. - Two more exit copies are checked. A blocked site may list only exits between its own two owners (or between the top and bottom maps of a relayering site), and a kept decoration's plane_source must be its tile on the plane. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

zoeyrose · 2026-10-18T18:29:05Z

Round 3 (Codex review of 808ea4a): two findings fixed in a1f5c17, one declined.

Source maps of unresolved exits (r4308148254): fixed. Every inventory entry's source map must belong to a group, whether or not the exit has a target.
Run the suite from tools/validate.sh (r4308148257): declined here. The scope is migration/placement-plan/scripts/, and per AGENTS.md a new validation path also needs a wrapper supply-chain inventory update in another repository. It is listed as a follow-up for the owner.
Dynamic exit targets (r4308148259): fixed, more broadly. The inventory now carries every fact the exit data problems come from (dynamic path, tile-link slot, target problem and authored target), and the verifier re-derives each data-problem entry field by field.

Also in a1f5c17: a blocked site may list only exits between its own owners (or its top and bottom maps), and a kept decoration's plane_source must be its plane tile.

Validation for a1f5c17:

test_placement_plan.py: 42 tests pass.
verify_plan.py: ok.
Regeneration from content 3ad20150da76: byte-identical across two runs. Only exit_inventory gains fields; REPORT.md and the PNGs are unchanged.
The radius-10 build verifies cleanly. The forced-failure build shows only its 17 expected placement errors.

zoeyrose · 2026-10-18T18:29:07Z

@codex review Please do a deep, thorough review of the whole diff and the code it touches and report every issue you find in this pass, including P2s, not only the most important few.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a1f5c1706d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Address the fourth Codex review round on #47. - Portal records: portal_reason_text is re-derived exactly, like the code and slope_reachable. - Stairwells: the shape follows from the ends (column or sloped, no other value), the kind is known and the ids run in one series per kind. max_bend, the owners (group, owner, other, connects: from the replaced exits), and the intermediate tiles' decoration (the exits' stair family) are re-derived. A stairwell must replace at least one exit. - Blocked sites: each must list an exit and have the shape of its ends, and a group's site belongs to the later placed end. The summary's stairwell and blocked-site counts are re-derived. - Groups: classifications and reasons must be known values, there is one overworld group, and the summary's group, map and classification counts are re-derived. - Placements: the offsets must equal the ideal spot plus the recorded shift and extra layers, and the ideal must follow from the main connection within the recorded vertical bounds. - The verifier's docstring now lists what it does not re-derive (the generator's explanations and evidence). The plan is unchanged; a rerun is byte-identical. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

zoeyrose · 2026-10-18T18:52:28Z

Round 4 (Codex review of a1f5c17): all six findings fixed in 0006dd7.

Main connection vs. placement (r4308221999): the ideal spot is re-derived from the main connection (within the recorded bounds), and the offsets from the ideal plus the recorded shift and extra layers.
Portal reason text (r4308222001): re-derived exactly. Every exit-record field is now either compared with the inventory or re-derived.
Stairwell shapes (r4308222004): derived from the ends (column / sloped only); kinds and per-kind id series are checked.
Classifications (r4308222008): known classification/reason pairs only, one overworld group, and the summary's group, map and classification counts re-derived.
Stairwell owners (r4308222011): group, owner, other and connects are re-derived from the replaced exits; max_bend and the intermediate decoration are checked too.
Empty blocked sites (r4308222013): each site lists at least one exit and has the shape of its ends; the summary's stairwell and site counts are re-derived.

To stop this from becoming field-by-field patching, the verifier's docstring now spells out its contract, including what it deliberately does not re-derive: the generator's explanations and evidence, which the migration does not act on.

Validation for 0006dd7:

test_placement_plan.py: 51 tests pass.
verify_plan.py: ok.
Regeneration from content 3ad20150da76: byte-identical; the plan is unchanged.
The radius-10 build verifies cleanly. The forced-failure build shows only its 17 expected placement errors.

zoeyrose · 2026-10-18T18:52:31Z

@codex review Please do a deep, thorough review of the whole diff and the code it touches and report every issue you find in this pass, including P2s, not only the most important few.

zoeyrose · 2026-10-18T19:12:50Z

@codex security review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0006dd7296

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-10-18T19:25:02Z

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 0006dd7296

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

You comment "@codex security review"
A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

Address the fifth Codex review round on #47. - The overworld group's map count and per-layer counts are re-derived from its cells, like the other groups' counts from their member lists. - Every resolved target in the exit inventory must lie on a tile of its target map. - A column stairwell may leave its column by at most settings.stairwell_max_bend_tiles, and the blocked-site chain search uses that setting instead of a hard-coded 2. The plan is unchanged; a rerun is byte-identical. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

zoeyrose · 2026-10-18T19:26:46Z

Round 5 (Codex review of 0006dd7): all three findings fixed in 9bc81aa.

Overworld map count (r4308381671): re-derived from cells_by_layer, together with its per-layer layers counts.
Resolved targets out of bounds (r4308381678): every resolved inventory target must lie on a tile of its target map, which also covers the record copies.
Bend limit (r4308381681): column stairwells may bend at most stairwell_max_bend_tiles, and the blocked-site chain search uses the same setting.

Validation for 9bc81aa:

test_placement_plan.py: 54 tests pass.
verify_plan.py: ok.
Regeneration: byte-identical; the plan is unchanged.
The radius-10 build verifies cleanly.

The Codex security review for 0006dd7 started after the re-request and was still running at this push.

zoeyrose · 2026-10-18T19:26:49Z

@codex review Please do a deep, thorough review of the whole diff and the code it touches and report every issue you find in this pass, including P2s, not only the most important few.

chatgpt-codex-connector · 2026-10-18T19:40:19Z

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: 9bc81aa748

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

zoeyrose · 2026-10-18T19:48:53Z

@codex security review

chatgpt-codex-connector · 2026-10-18T19:57:55Z

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 9bc81aa748

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

You comment "@codex security review"
A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

zoeyrose · 2026-10-18T19:58:59Z

@codex review Please do a deep, thorough review of the whole diff and the code it touches and report every issue you find in this pass, including P2s, not only the most important few.

zoeyrose · 2026-10-18T22:55:32Z

@codex review Please do a deep, thorough review of the whole diff and the code it touches and report every issue you find in this pass, including P2s, not only the most important few.

63a5fd6 committed scripts/__pycache__/build_plan.cpython-314.pyc. It was written when verify_plan.py --content imported the generator, before any module disabled bytecode writing. That broke tools/test_writable_source.py in CI. Remove the file, and have verify_plan.py set sys.dont_write_bytecode at import, as build_plan.py and the tests already do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

chatgpt-codex-connector

💡 Codex Review

content-toolkit/migration/placement-plan/scripts/__pycache__/build_plan.cpython-314.pyc

Line 1 in 63a5fd6

+�

Remove the tracked Python bytecode cache

The committed .pyc is a generated CPython 3.14 cache rather than source: it embeds the originating checkout path and depends on interpreter version and source metadata. A normal import may either execute it when its header happens to validate or rewrite the tracked file when it does not, making behavior and worktree cleanliness depend on cache state. Remove it and ignore __pycache__/*.pyc.

AGENTS.md reference: AGENTS.md:L36-L39

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…s in CI Address the Codex review of 63a5fd6 on #47. - verify() turns any exception from a malformed plan (a missing key, a null where a list belongs) into a failed result rather than a traceback. verify_plan.py --content reports a checkout it cannot rebuild from the same way. - Placement offsets, ideal, shift and extra layers must be integers (booleans and floats are rejected). - The plan's schema is now atrinik-overworld-placement-plan/4, since exit_inventory and the failed-placement bookkeeping are required, and the verifier accepts only that schema. - tools/validate.sh runs test_placement_plan.py after the Cargo steps, like the other tools/test_*.py suites, so CI exercises the verifier's protections. The content-fidelity tests skip there. Regenerated from content 3ad20150da76: only the schema string changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

zoeyrose · 2026-10-18T23:23:38Z

Round 13 (Codex review of 63a5fd6) and a CI failure: fixed in 791b2c0 and cc94be1.

CI failure: 63a5fd6 committed scripts/__pycache__/build_plan.cpython-314.pyc, written when --content imported the generator. tools/test_writable_source.py caught it. 791b2c0 removes the file, and verify_plan.py now disables bytecode writing at import.

Crash on malformed values (r4309179996): verify() turns any exception from a malformed plan into a failed result, and --content does the same for a checkout it cannot rebuild from.
Run the suite in the aggregate check (r4309180003, P1): tools/validate.sh now runs test_placement_plan.py after the Cargo steps. Correction to round 3: I declined this partly on the grounds that the wrapper supply-chain inventory would need an update. It doesn't: that inventory's content-toolkit evidence covers only the Cargo manifests, workflows and toolchain. This is the one change outside migration/placement-plan/.
Booleans as placement numbers (r4309180008): placement offsets, ideal, shift and extra layers must be exact integers.
Schema version (r4309180012): the plan is now atrinik-overworld-placement-plan/4, and the verifier accepts only that schema.

Validation for cc94be1:

tools/validate.sh (with the CI-pinned syft): passes, including the suite (77 tests, 2 content tests skipped).
With PLACEMENT_PLAN_CONTENT set, all 77 pass.
verify_plan.py --content: ok.
build_plan.py --check: all 34 artifacts byte-identical.

zoeyrose · 2026-10-18T23:23:43Z

@codex review Please do a deep, thorough review of the whole diff and the code it touches and report every issue you find in this pass, including P2s, not only the most important few.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cc94be1550

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…risons Address the Codex review of cc94be1 on #47, fixing each class rather than each field. - One pass over the plan rejects floats anywhere, and booleans outside the known flag keys. Python's true == 1 can then no longer stand in for a distance, a layer or a count in any JSON-only comparison (layer_delta and xy_distance included). - The fidelity comparison checks JSON type for type in place, without serializing both plans. verify_plan.py refuses a plan file over 64 MiB before parsing it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

zoeyrose · 2026-10-18T23:55:59Z

Round 14 (Codex review of cc94be1): both findings fixed in 85ed583, as classes.

Booleans as distances (r4309281267): one pass rejects floats anywhere and booleans outside the known flag keys, so true can't stand in for any integer in the JSON-only check.
Memory in the fidelity comparison (r4309281269): plans are compared type for type in place (same_json), and plan files over 64 MiB are refused before parsing.

Validation for 85ed583:

tools/validate.sh: passes, including the suite (81 tests, 3 content tests skipped).
With PLACEMENT_PLAN_CONTENT set, all 81 pass.
verify_plan.py --content: ok.
build_plan.py --check: all 34 artifacts byte-identical.
The radius-10 build verifies cleanly.

zoeyrose · 2026-10-18T23:56:03Z

@codex review Please do a deep, thorough review of the whole diff and the code it touches and report every issue you find in this pass, including P2s, not only the most important few.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 85ed583018

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Address the Codex review of 85ed583 on #47. Each of the suite's 134 verifications re-runs the full verifier on the 3 MB plan, about 1.2 s locally, so the serial suite took about 165 s here and about 10 min in Codex's environment. The tests are independent and CPU-bound, so the suite's entry point now runs one test per task in up to 8 forked worker processes, with --jobs 1 for serial. Each case still verifies its own tampered plan in isolation. Locally the suite takes 29 s with 8 workers and 48 s with 4, the size of CI's runners. Failures are reported with their unittest output and a nonzero exit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

zoeyrose · 2026-10-19T00:27:17Z

Round 15 (Codex review of 85ed583): the one finding fixed in 24e1329.

Suite runtime in tools/validate.sh (r4309376395): each tamper case runs the full verifier on the real plan (134 verifications, about 1.2 s each here). A smaller fixture would need a second, synthetic content tree for the generator, and combining cases in shared plans would let one mask another. So the cases stay isolated, and the suite's entry point now runs them in up to 8 forked worker processes (--jobs N). It takes 29 s with 8 workers and 48 s with 4 (CI's runner size), down from 165 s serial.

Validation for 24e1329:

tools/validate.sh: passes (suite: 81 tests, 3 content tests skipped, 29 s).
The failure path reports and exits nonzero.
--jobs 1 works.

zoeyrose · 2026-10-19T00:27:21Z

@codex review Please do a deep, thorough review of the whole diff and the code it touches and report every issue you find in this pass, including P2s, not only the most important few.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 24e1329249

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…heck Address the Codex review of 24e1329 on #47. Each test verified its own tampered plan in full, and about 80% of a verification is the relayering re-derivation (242,824 overworld tiles), whose inputs almost no test touches. - relayering_rules() is now a pure function of explicit inputs: the overworld rows, the relayering record, the overworld's internal exits, the seam pieces' exits that have a record, and four settings. _relayering() memoizes the last result in the process, keyed by a SHA-256 of the canonical JSON of exactly those inputs, so the results are the same with or without the cache. A test asserts that each input is in the key, and that a changed relayering input is recomputed. - The occupancy pass finds '#' tiles with str.find and computes bounding boxes once per owner instead of per tile. The number-type pass checks first without building paths. - The test runner verifies the committed plan before forking, so every worker starts with the cache warm. verify_plan.py's output is byte-identical. A verification of an unchanged relayering takes 0.22 s instead of 1.15 s. The suite takes 39 s serially (was 166 s), 14 s with 4 workers and 10 s with 8. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

zoeyrose · 2026-10-19T00:57:19Z

Round 16 (Codex review of 24e1329): fixed in 3d04cb7.

Repeated full-plan work in the suite (r4309454294): on CI, 24e1329's suite took 185 s with 4 workers. About 80% of every verification was the relayering re-derivation, which almost no test touches. It is now a pure function (relayering_rules), memoized in-process on a SHA-256 of exactly its inputs, so results are the same with or without the cache, and a test proves each input is in the key. The occupancy pass is also cheaper. Output is byte-identical. The suite takes 39 s serially (was 166 s), 14 s with 4 workers and 10 s with 8. Each tamper case still verifies its own plan in full.

Validation for 3d04cb7:

tools/validate.sh: passes (suite: 82 tests in 9.6 s, 3 skipped).
With PLACEMENT_PLAN_CONTENT set, all 82 pass.
verify_plan.py --content: ok.
build_plan.py --check: all 34 artifacts byte-identical.
The radius-10 and forced-failure builds behave as before.

zoeyrose · 2026-10-19T00:57:24Z

@codex review Please do a deep, thorough review of the whole diff and the code it touches and report every issue you find in this pass, including P2s, not only the most important few.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3d04cb7f87

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Address the Codex review of 3d04cb7 on #47. - report.py lists every wired exit, including one wired between two overworld maps (internal_exits), which the Wired exits section used to omit. The crossing-stairs sentence counts only crossing wired exits that became stairs, not summary.wired_exits. A report test with such an exit fails on the old code. - verify() requires the plan to identify its input: a non-empty content_root, a 40-hex content_git_head, a boolean content_worktree_clean and a well-formed inputs digest. A plan that has lost its content_root no longer passes because --content ignores the value. The plan and REPORT.md are unchanged; --check passes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

zoeyrose · 2026-10-19T01:20:04Z

Round 17 (Codex review of 3d04cb7): both findings fixed in 7564152. Neither is another per-field fidelity case:

Missing content_root (r4309582005): verify() now checks the plan's whole input identity (content_root, a 40-hex content_git_head, content_worktree_clean, the inputs digest), so --content excluding the path's value can't hide a plan that lost it.
Wired count in the report (r4309582008): the crossing-stairs sentence counts crossing wired stairs from the records, and the Wired exits section now also lists exits wired within the overworld. A new report test fails on the old code.

Validation for 7564152:

tools/validate.sh: passes (suite: 84 tests in 9.9 s, 3 skipped).
With PLACEMENT_PLAN_CONTENT set, all 84 pass.
verify_plan.py --content: ok.
build_plan.py --check: all 34 artifacts byte-identical; the plan and REPORT.md are unchanged.

zoeyrose · 2026-10-19T01:20:10Z

@codex review Please do a deep, thorough review of the whole diff and the code it touches and report every issue you find in this pass, including P2s, not only the most important few.

chatgpt-codex-connector · 2026-10-19T01:27:46Z

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: 75641528cd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

zoeyrose · 2026-10-19T01:40:41Z

@codex security review

chatgpt-codex-connector · 2026-10-19T01:44:58Z

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 75641528cd

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

You comment "@codex security review"
A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

chatgpt-codex-connector Bot reviewed Oct 18, 2026

View reviewed changes

Comment thread migration/placement-plan/scripts/verify_plan.py Outdated

chatgpt-codex-connector Bot reviewed Oct 18, 2026

View reviewed changes

Comment thread migration/placement-plan/scripts/verify_plan.py

Comment thread migration/placement-plan/scripts/test_placement_plan.py

Comment thread migration/placement-plan/scripts/build_plan.py

chatgpt-codex-connector Bot reviewed Oct 18, 2026

View reviewed changes

Comment thread migration/placement-plan/scripts/verify_plan.py Outdated

Comment thread migration/placement-plan/scripts/verify_plan.py Outdated

Comment thread migration/placement-plan/scripts/verify_plan.py

chatgpt-codex-connector Bot reviewed Oct 18, 2026

View reviewed changes

Comment thread migration/placement-plan/scripts/verify_plan.py

Comment thread migration/placement-plan/scripts/test_placement_plan.py Outdated

Comment thread migration/placement-plan/scripts/verify_plan.py

Comment thread migration/placement-plan/scripts/build_plan.py

chatgpt-codex-connector Bot reviewed Oct 18, 2026

View reviewed changes

Comment thread migration/placement-plan/scripts/verify_plan.py

Comment thread migration/placement-plan/scripts/verify_plan.py Outdated

chatgpt-codex-connector Bot reviewed Oct 19, 2026

View reviewed changes

Comment thread tools/validate.sh

chatgpt-codex-connector Bot reviewed Oct 19, 2026

View reviewed changes

Comment thread migration/placement-plan/scripts/test_placement_plan.py

chatgpt-codex-connector Bot reviewed Oct 19, 2026

View reviewed changes

Comment thread migration/placement-plan/scripts/verify_plan.py

Comment thread migration/placement-plan/scripts/build_plan.py

zoeyrose marked this pull request as ready for review October 19, 2026 01:46

zoeyrose self-assigned this Oct 19, 2026

zoeyrose merged commit bbe08d2 into main Oct 19, 2026
12 checks passed

zoeyrose deleted the fix/placement-plan-verifier-gaps branch October 19, 2026 05:14

zoeyrose mentioned this pull request Oct 20, 2026

feat(placement-plan): apply the owner decisions of 2026-10-19 #48

Merged

Conversation

zoeyrose commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Implementation / behavior

Validation

Limitations / follow-up

Uh oh!

github-actions Bot commented Oct 18, 2026

Dependency Review

Scanned Files

Uh oh!

zoeyrose commented Oct 18, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zoeyrose commented Oct 18, 2026

Uh oh!

zoeyrose commented Oct 18, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

zoeyrose commented Oct 18, 2026

Uh oh!

zoeyrose commented Oct 18, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zoeyrose commented Oct 18, 2026

Uh oh!

zoeyrose commented Oct 18, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zoeyrose commented Oct 18, 2026

Uh oh!

zoeyrose commented Oct 18, 2026

Uh oh!

zoeyrose commented Oct 18, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Oct 18, 2026

🛡️ Codex Security Review

Uh oh!

zoeyrose commented Oct 18, 2026

Uh oh!

zoeyrose commented Oct 18, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Oct 18, 2026 •

edited

Loading