Measure what the blueprint GUI's Grid position field does - #249
Merged
Conversation
PR #243 ships a getGridPositionDisplay() built on the rule that setting Grid position to T shifts the exported content so -floor(min position over entities and tiles) equals T. tools/oracle/fixtures/blueprint-grid-position.json appears to refute exactly that, scoring the premise 0 of 2 with entitiesMovedOn: 0 on 2.0.77, and factorio-oracle/docs/method.md cites it as settled. They are not about the same field. The earlier capture set blueprint_position_relative_to_grid, and the game's own locale separates the two in one line - core.cfg:274, "Grid position and blueprint grid position coordinates need to be either all even or all odd". So the old fixture measured the neighbour. Its answer stands and is not an answer to this question. Interactive because the cheap sources were exhausted first, per docs/order-of-attack.md: LuaItemCommon on 2.0.77 exposes exactly three blueprint snapping attributes and none is this one, create_blueprint takes no anchor parameter, and the game's own tooltip says the field is set by shift + left-click in the preview. There is no script that can set it. Runs on the shared factorio-oracle CLI rather than as a 19th script here, which is what issue #235's "new probes only" rule asks of the next probe anyone writes. The analysis half stays here, since it compares the game against this editor's own reimplementation. The layout separates three readings of "the minimum corner" on different axes at once: x-min is decided by a 3x3 entity whose centre and edge floor to different integers, and y-min by a tile clear of everything else, so whether tiles count is visible rather than assumed. Blueprint.ts reads centres and includes tiles, and its own comment admits the footprint half is untested because every case measured so far used 1x1 entities where the two agree. The content does not start at the origin and the session asks for a second change on top of a first, because an absolute target and a relative nudge agree whenever the corner is already 0. Three scripted controls, each able to fail while the hypothesis holds: instrument-repeat catches non-deterministic export, positive-shift proves the comparison can see a shift at all, and rival-field sets blueprint_position_relative_to_grid on the same rig so the two fields can be told apart rather than merely asserted to differ. Script-set state is tagged and excluded from scoring by tag, not by value. The analysis was validated against a synthetic stream before any game time: all four readings compute as hand-derived and all differ; planting a world where entity-centres-plus-tiles is true leaves exactly that reading surviving; and two mutations each fail exactly one control and exit non-zero. A session capturing no scored step reports "unmeasured" rather than four vacuously surviving readings, which is what every() over an empty list had produced. Refs #235
--version on its own selects nothing here: a bare discovery finds only the Steam 2.1.14, so there is no 2.0.77 for it to filter down to. --factorio puts that root at the front of the candidate list and --version is then a guard, so picking the wrong game is impossible rather than merely unlikely. Measured - installs list takes no selector at all, which is why this was not caught by running it. SESSION.md is the follow-along half: one-liners, the steps in order, and what each survivingReadings value would mean. README.md keeps the reasoning.
Runs the interactive probe on 2.0.77 and records the answer. The field writes no key into the export at all - it translates the entity and tile coordinates instead, which is why runtime-api.json exposes nothing that can reach it and why this needed a person at the keyboard. survivingReadings is entityEdgesAndTiles, with all three controls passing and every rival reading killed on all three scored rows. So the rule is -floor(min corner) = T, the corner taken over entity **edges** and tiles. It is an absolute target rather than a relative nudge: setting 8,9 on top of 3,5 moved the corner to -8,-9, not to -11,-14. That refutes the apparent conflict with blueprint-grid-position.json, which set blueprint_position_relative_to_grid. The panel carries three X/Y pairs and two of them were on screen at once holding 3,5 and 9,10 - separate fields, so the older fixture's answer stands and was never about this one. Its supersedes block says so. Four fixes the run itself turned up: - The run command needs the repo root as its cwd, since probe.json names control.lua by a repo-relative path that the CLI reads verbatim. An absolute --probe from elsewhere fails naming the Lua file rather than the cause. - Session step 6 said to pick Absolute, which is already the default the moment Snap to grid is ticked, and described its pair as the Grid position one. There are three pairs, not two. - The analyzer now emits stepsNotCaptured. An interactive probe ends when the person stops playing, so a fixture that simply omits a step reads as "this step does not exist" rather than "it was not run". - tools/oracle/fixtures is exempt from oxfmt. The generator and the formatter disagreed on the canonical form of a generated file, so whichever ran last won and the other reported a failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RiP5JRvEpzDXM4AY5ouGN
Records what the blueprint GUI's "Grid position" field does, measured on 2.0.77: -floor(min corner) = T over entity edges and tiles, an absolute target rather than a relative nudge. The lesson worth more than the finding is that this field is not the one the paragraph above it measures. The panel carries three X/Y pairs and Grid position writes no key into the export at all, so blueprint-snapping .json and blueprint-grid-position-gui.json are both right about different questions. Reading one as the other is what sent a session off to settle a contradiction that did not exist. Also notes that getGridPositionDisplay() reads centres where the game reads edges, that the editor's own solve hides it by using the same formula on both sides, and that the measurement cannot separate the tile footprint edge from the collision box edge - which is the thing that decides the fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RiP5JRvEpzDXM4AY5ouGN
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Comment on lines
+160
to
+169
| const base = r1 | ||
| const shifted = byLabel.get('control-positive-shift') | ||
| let movedOk = false | ||
| let movedDetail = 'missing the shift control' | ||
| if (base && shifted) { | ||
| const a = asArray(base.entities).map(e => `${e.position.x},${e.position.y}`) | ||
| const b = asArray(shifted.entities).map(e => `${e.position.x},${e.position.y}`) | ||
| movedOk = a.join('|') !== b.join('|') | ||
| movedDetail = `baseline ${a.join(' ')} vs script-shifted ${b.join(' ')}` | ||
| } |
There was a problem hiding this comment.
Bug: the positive-shift control validates a different data path than the one actually used for scoring, so it can't detect the failure it exists to guard against.
This control checks base.entities / shifted.entities, which come straight from bp.get_blueprint_entities() (see control.lua's capture()). But every scored row is read from the decoded export string instead:
The README states this control's job is to prove "the comparison can see a shift at all" — but the comparison that actually matters for survivingReadings is decodeBlueprintString(row.export), a completely separate code path from get_blueprint_entities(). If export_stack() were ever stale/unresponsive while get_blueprint_entities() still worked, this control would pass, instrument-repeat would also trivially pass (identical exports), and the whole probe would report a confident-but-wrong survivingReadings: [] ("all four candidates are wrong") when the real cause is a blind instrument on the path that's actually scored.
Fix: build a/b from decodeBlueprintString(base.export).blueprint?.entities and decodeBlueprintString(shifted.export).blueprint?.entities, the same way the scored rows are read, rather than from the raw .entities field.
Comment on lines
+33
to
+44
| /** | ||
| * The three entities this probe's layout uses, and their tile footprints. | ||
| * Hardcoded rather than read from `data.json`, because the layout is owned by | ||
| * the probe next door rather than discovered: these are the sizes it placed, | ||
| * and a `data.json` that disagreed would mean the exporter had drifted, not | ||
| * that the measurement should change. Cross-checked below when data.json is | ||
| * present, and reported rather than silently trusted. | ||
| */ | ||
| const SIZES = { | ||
| 'assembling-machine-1': { w: 3, h: 3 }, | ||
| 'wooden-chest': { w: 1, h: 1 }, | ||
| } |
There was a problem hiding this comment.
Bug: this comment claims a cross-check against data.json that doesn't exist anywhere in this file, and the actual fallback it's describing is silent rather than reported.
data.json is referenced nowhere else in this file (grep confirms it appears only in comments). The last sentence — "Cross-checked below when data.json is present, and reported rather than silently trusted" — describes functionality that was never written. (The comment also says "three entities" for a two-entry table.)
What actually happens is the opposite of "reported": the edge computation silently falls back to size 1x1 for any unrecognized entity name —
— which collapses entityEdgesAndTiles/entityEdgesOnly toward the entityCentres* readings with no warning. Since distinguishing edge-reads from centre-reads is the entire point of this probe, a probe-layout change or a new entity name landing in the export without a matching SIZES entry would silently corrupt the exact discrimination this file exists to make — with controlsAllPassed: true and no indication anything was wrong.
Comment on lines
+264
to
+273
| local inv = player.get_main_inventory() | ||
| inv.insert { name = 'blueprint', count = 1 } | ||
| local bp = nil | ||
| for i = 1, #inv do | ||
| local s = inv[i] | ||
| if s.valid_for_read and s.is_blueprint and not s.is_blueprint_setup() then | ||
| bp = s | ||
| break | ||
| end | ||
| end |
There was a problem hiding this comment.
Bug: setup() never clears a previously set-up blueprint before inserting a new one, so /gp-reset can make every subsequent capture silently record stale data from the prior session.
This loop deliberately skips any slot where is_blueprint_setup() is already true (not s.is_blueprint_setup()), leaving the old set-up blueprint sitting in an earlier inventory slot. find_bp() (used by capture() when no stack is passed) checks the cursor first, then falls back to the first set-up blueprint by slot order:
Since setup() never places the freshly created blueprint into the cursor, after /gp-reset the inventory holds two set-up blueprints and find_bp() keeps returning the old one (lower slot index) for every /gp-cap — unless the operator happens to be holding the new one. Each capture still prints captured: <label> as if it succeeded, so the corruption is silent and would land directly in the committed fixture. setup() should clear/remove any pre-existing set-up blueprint (or place the new one in the cursor) before filling a new one.
Comment on lines
+68
to
+73
| function readStream(workDir, run) { | ||
| const path = | ||
| run?.scriptOutput !== undefined | ||
| ? join(run.scriptOutput, 'grid-position-gui.jsonl') | ||
| : join(workDir, 'write', 'script-output', 'grid-position-gui.jsonl') | ||
| if (!existsSync(path)) { |
There was a problem hiding this comment.
Bug: with the documented --run <report.json>-only invocation (no work-dir positional — see README.md/SESSION.md "Reading the result"), if the run report lacks scriptOutput, this throws a raw TypeError [ERR_INVALID_ARG_TYPE] from path.join(undefined, ...) instead of the intended "No capture stream at ..." error a few lines below.
The usage guard only requires workDir OR runPath, not both:
So --run alone leaves workDir as undefined, and if run.scriptOutput is also missing (a malformed/partial run report), the ternary's else-branch calls join(undefined, 'write', 'script-output', ...) before the existsSync check ever runs.
Suggested change
| function readStream(workDir, run) { | |
| const path = | |
| run?.scriptOutput !== undefined | |
| ? join(run.scriptOutput, 'grid-position-gui.jsonl') | |
| : join(workDir, 'write', 'script-output', 'grid-position-gui.jsonl') | |
| if (!existsSync(path)) { | |
| function readStream(workDir, run) { | |
| if (run?.scriptOutput === undefined && workDir === undefined) { | |
| throw new Error( | |
| 'No script-output directory: the run report carries no scriptOutput and no work-dir was given.' | |
| ) | |
| } | |
| const path = | |
| run?.scriptOutput !== undefined | |
| ? join(run.scriptOutput, 'grid-position-gui.jsonl') | |
| : join(workDir, 'write', 'script-output', 'grid-position-gui.jsonl') | |
| if (!existsSync(path)) { |
The four are all from the review on #249, and all four are real. Three are instrument bugs in the sense docs/method.md means: each would have produced a confident answer with nothing flagged. - The positive-shift control compared `get_blueprint_entities()` output while every scored row is read from the decoded export. Two different instruments, and it validated the one that is not scored. A stale `export_stack()` would have left this control passing, instrument- repeat trivially passing on two identical stale exports, and the probe reporting `survivingReadings: []`. The rival-field control ten lines below already read the export, so the file disagreed with itself. - An entity with no footprint entry silently defaulted to 1x1, which drags the edge readings towards the centre readings - the exact discrimination this probe exists for - with controlsAllPassed true. Now a control that fails, and mutation-checked: dropping wooden-chest from the table exits 1. - `setup()` never cleared an existing set-up blueprint, while `find_bp()` takes the first set-up one by slot order. So after /gp-reset every capture silently read the previous session's blueprint and still printed `captured: <label>`. This run never reset, so the committed fixture is unaffected. - `readStream` threw ERR_INVALID_ARG_TYPE on a run report with no scriptOutput, burying the real cause under a stack trace about paths. The comment describing a data.json cross-check was describing code that was never written, and said "three entities" over a two-entry table. And the substantive part: a fifth reading, entityCollisionEdgesAndTiles. Run 1 said the corner is read from an edge and could not say which, because the footprint edge and the box edge floor alike for everything it placed - assembling-machine-1 is 9.0 against 9.3. Adding the reading makes the fixture say that outright: run 1's own data now leaves **two** survivors rather than asserting one. Run 2's layout settles it. half-diagonal-rail is close to the only entity that can: the editor derives a footprint by ceiling the collision box, so a box escapes its footprint only where a declared tile_width overrides that - five of 155 entities - and this is the only one splitting all three readings on one axis (centre 20, footprint edge 19, box edge 17.764). Three questions and two axes, so the tiles moved to own x while the rail owns y. Its orientation is the known risk, and it fails visibly rather than silently now that the fixture records the layout that produced it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RiP5JRvEpzDXM4AY5ouGN
2 tasks
…g about rails Run 2's layout rested on a half-diagonal-rail producing three distinct floors on one axis, with numbers taken from data.json. A headless create run checks that before anyone spends a session on it - no player is needed, so none of this had to be interactive. Three findings, each of which would have corrupted run 2 silently: - The box does not rotate. All 16 directions give one identical box, folding to stored directions 0/2/4/6. The orientation fear that made this probe worth writing was unfounded, which is still worth having measured rather than assumed. - The rail snaps. Asked for (20,20), the game places it at (21,21), so every predicted number was for a position that cannot exist. The layout now asks for 21 and predicts centre 21, footprint edge 20, box edge 19.102 - still three distinct floors. - data.json disagrees with the running game about collision boxes, and only about rails. Of 155 entities 139 agree to within one 1/256 step, which is just Factorio's position quantum and not a finding; all 16 that exceed it are rails, including every dummy- and elevated- variant. legacy-curved-rail is out by 1.45 tiles and its runtime box is not symmetric where data.json says it is. That last one is why the grid-position analyzer now carries the game's number for this rail rather than the exporter's. It also bears on the rail geometry CLAUDE.md already records as wrong in both directions (#133, #142): the editor's own data disagrees with the game there, and nowhere else. Two method notes in the probe README, both of which cost real time. A ]] inside a --[[ Lua comment closes it, so a header explaining a collision box in Lua table notation silently turned the rest of the file into code and the mod never registered its on_init - reported as "no oracle-dump.json was written", the same message a factorio_version mismatch gives. What located it was running the CLI's own example probe as a control, and the tell in the log is a line that is absent: a working run prints `Checksum for script __<mod>__/control.lua`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RiP5JRvEpzDXM4AY5ouGN
Running the analysis before the probe gave a raw node:fs stack trace naming the path and nothing else. It is the ordinary mistake - the analysis is step 3 of SESSION.md and the probe is step 1 - and it happened on the first attempt at run 2. Same shape as the readStream guard from #249's review, one function over: the file that reports what a probe measured should not fail in a way that looks like the probe measured something wrong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RiP5JRvEpzDXM4AY5ouGN
SESSION.md tells the operator to use the nearest value the game accepts
when one is refused, and record what was used. The analysis could only
score the three labels it happened to know, so following that
instruction produced an unscored row and a session that measured
nothing.
Not hypothetical: the parity rule ("Grid position and blueprint grid
position coordinates need to be either all even or all odd") refused 3,5
outright on run 2's layout, having accepted it on run 1's. Any
gridpos-<x>-<y> label is now read from its own numbers, and a
substitution no longer reports the asked-for label as a skipped step.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019RiP5JRvEpzDXM4AY5ouGN
Run 2 settles the half PR #243 left open. Two scored rows, four controls passing: the game reads each entity's **tile footprint edge** and counts tiles. Each wrong reading misses by exactly one tile on y in opposite directions, and ignoring tiles misses by seven on x. The methodological note is the one worth keeping. Run 1 reported a single surviving rule and it was overstated - the candidate that would have tied with it had not been written down, so "one survivor" meant "one of the rules I thought of". Adding the fifth reading made the same data report two, which is the honest answer. The rival list is part of the instrument. Also records #251 and #252, both found on the way rather than looked for: data.json disagrees with the running game about every rail's collision box and nothing else's, and the game validates this field three ways where the editor validates it none, having refused 3,5 on one layout after accepting it on another. And the Lua trap that cost two runs: a ]] inside a --[[ comment closes it, so a header explaining a collision box in table notation turned the rest of the file into code and the mod never registered its on_init. The run reports the same message a factorio_version mismatch gives. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RiP5JRvEpzDXM4AY5ouGN
Comment on lines
+64
to
+72
| `assembling-machine-1` is 9.0 against 9.3. | ||
|
|
||
| **Run 2** replaces the assembling machine with a `half-diagonal-rail`, which is | ||
| close to the only entity that can settle it: | ||
|
|
||
| | Axis | Decided by | Separates | | ||
| | ---- | ------------------------------ | ---------------------------------------------------------- | | ||
| | y | a `half-diagonal-rail` at y=20 | centre **20**, footprint edge **19**, box edge **17.764** | | ||
| | x | stone-path tiles at x=2 | whether tiles count, against the nearest entity reading, 9 | |
There was a problem hiding this comment.
Two contradictions between this README and the rest of the PR's own diff:
- Line 66 says Run 2 "replaces the assembling machine with a
half-diagonal-rail" — butcontrol.lua'sLAYOUTkeepsassembling-machine-1(moved to a new position) and adds the rail as a 4th entity; it's an addition, not a replacement. - The table on line 71 predicts "a
half-diagonal-railat y=20 | centre 20, footprint edge 19, box edge 17.764". Butcontrol.luaactually places the rail aty = 21(it snaps: "asking for (20, 20) places it at (21, 21)"), and its own header comment predicts "centre 21, footprint edge 20, box edge 19.102" — using the measured runtime collision box (-1.8984375) from the siblingprobe-rail-box-orientation, not the staledata.jsonvalue (-2.236) that produces 17.764.
This README is the operator's reference while running the probe, and it explicitly says a wrong prediction "surfaces as y readings matching none of the three predictions ... It is not silent." A reader following these numbers against an actual y=21 run would see all three predictions miss and could misread a correct run as a failed one.
Suggest updating line 71 to a half-diagonal-rail at y=21 | centre 21, footprint edge 20, box edge 19.102 and rewording line 66 to describe the rail as added alongside the machine rather than replacing it.
Comment on lines
+390
to
+405
| // Which steps the session never reached. An interactive probe ends when the | ||
| // person stops playing, so a fixture that simply omits them reads as "these | ||
| // steps do not exist" rather than "these steps were not run" - the silent | ||
| // cap docs/method.md warns about. Named here so a reader can tell a gap | ||
| // from a finding. | ||
| const substituted = cases.some( | ||
| c => c.label.startsWith('gridpos-') && EXPECTED[c.label] === undefined | ||
| ) | ||
| const stepsNotCaptured = Object.keys(EXPECTED).filter(label => { | ||
| if (cases.some(c => c.label === label)) return false | ||
| // A refused value means the operator captured `gridpos-4-6` in place of | ||
| // `gridpos-3-5`. Reporting the asked-for label as "not captured" would | ||
| // read as a skipped step rather than a substituted one. | ||
| if (substituted && label.startsWith('gridpos-')) return false | ||
| return true | ||
| }) |
There was a problem hiding this comment.
expectedFor() (line 222) parses a capture label with an anchored regex, /^gridpos-(-?\d+)-(-?\d+)$/, but substituted here uses a looser test — any label that startsWith('gridpos-') and isn't an exact key in EXPECTED sets substituted = true. That flag then suppresses every gridpos-* step from stepsNotCaptured (line 403), not just the one actually substituted.
This is live in the committed fixture: blueprint-grid-position-gui.json contains a case labeled gridpos-3-5-saved (a re-capture, not a substituted value), which trips substituted = true for the wrong reason. It's harmless in this particular run because all three real gridpos-* steps were captured, but on a run where the operator both uses a label like this and genuinely skips a gridpos-* step (e.g. the "do not skip this one" gridpos-8-9), that missing step would silently disappear from stepsNotCaptured — exactly the silent-gap failure the comment above this block says it exists to prevent.
Consider deriving substituted from labels that actually parse via expectedFor's own regex rather than a bare startsWith, and/or pairing each substitution with the specific missing key it replaces instead of suppressing the whole gridpos-* class.
| requestedPosition: dump.requested_position, | ||
| actualPosition: placed[0]?.position, | ||
| storedDirections: [...new Set(placed.map(d => d.stored_direction))].sort((a, b) => a - b), | ||
| runtimeCollisionBox: box(dump.directions.find(d => d.created).bounding_box), |
There was a problem hiding this comment.
dump.directions.find(d => d.created) returns undefined when no direction produced a placement (e.g. every create_entity call failed, or the Lua-side pcall raised before row.created was ever set). Accessing .bounding_box on that undefined throws a TypeError here, before controlsAllPassed is checked and reported at the bottom of the script. That's exactly the total-failure case the "every direction produced a placement" / "the probe recorded no Lua errors" controls exist to report gracefully — instead the script crashes with a stack trace naming neither the probe nor the failed control.
Note actualPosition: placed[0]?.position two lines above already guards the identical empty case with optional chaining; this is the one read that doesn't.
Suggested change
| runtimeCollisionBox: box(dump.directions.find(d => d.created).bounding_box), | |
| runtimeCollisionBox: placed[0] !== undefined ? box(placed[0].bounding_box) : null, |
wormeyman
added a commit
that referenced
this pull request
Aug 22, 2026
) PR #249 got seven review findings. Four arrived while it was open and were fixed in 2809e8d. The other three landed three minutes and thirty-nine seconds after the squash merge, so they shipped. `analyze-rail-box-orientation.mjs` read `.bounding_box` off `dump.directions.find(d => d.created)` without guarding the undefined. That is the total-failure case - no direction placed anything, or the Lua pcall raised before `created` was set - and it is exactly what the "every direction produced a placement" and "no Lua errors" controls exist to report. Instead it threw a TypeError while building the fixture literal, which happens before `controlsAllPassed` is consulted, so the run died naming neither the probe nor the failed control. `actualPosition` two lines up already guarded the same emptiness. Regenerating the fixture from the original dump gives a byte-identical file, so the guard changes nothing on real data. `analyze-blueprint-grid-position-gui.mjs` suppressed a whole class of steps to excuse one substitution. The game refuses some grid positions, the session says to substitute the nearest accepted value, and the analyzer hid the asked-for label so that would not read as a skipped step. But it used one boolean over every `gridpos-` label, so two substitutions excused three missing steps and `gridpos-0-0` disappeared from `stepsNotCaptured` - a genuinely skipped step, and the row that establishes the non-default check, vanishing from the list whose only job is to name what was skipped. That is the silent cap the field was written to prevent, in the code written to prevent it. One substitution now excuses one missing step, counted and spent in the order SESSION.md walks them. The `gridpos-` pattern moves into a named constant, because `expectedFor` and this filter have to agree about what one is, and the looser copy was winning. The fixture is regenerated from the original run's capture stream rather than edited, and it reproduces byte for byte apart from the restored `gridpos-0-0`. The probe README predicted the wrong numbers for run 2. It said the rail sits at y=20 with a collision-box edge of 17.764, and it said run 2 replaces the assembling machine. The rail is at 21 - `probe-rail-box-orientation` measured that a rail requested at (20, 20) is placed at (21, 21) - its box edge is 19.102 off the measured runtime box, not the 17.764 that `data.json`'s disputed -2.236 gives from the wrong position, and the machine stays in the layout and merely moves. That file is what an operator reads while the run is in front of them, and it says a bad prediction "is not silent" - true, but a bad prediction looks identical to a bad run. Claude-Session: https://claude.ai/code/session_013SHoE9cH9kd6vtpHM1PzZ1 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
wormeyman
added a commit
that referenced
this pull request
Aug 25, 2026
… notes (#263) * Skip the Claude review workflow on pull requests from forks GitHub withholds repository secrets from a `pull_request` event raised by a fork, so `secrets.CLAUDE_CODE_OAUTH_TOKEN` resolves to an empty string and the action fails every time. Measured across the open backlog: `claude-review` failed on all five fork PRs (#227, #242, #243, #258, and #249 before it merged) and passed on both in-repo ones (#257, #260). That is the whole pattern - it is not a misconfiguration the workflow can fix, it is what the event is for. The failure blocks nothing, which is the problem. Every fork PR opens with a red X, and a check that is always red is a check nobody reads - so a real failure in it would be missed. A job-level `if` turns it grey instead. The alternative is `pull_request_target`, which does get fork PRs reviewed but hands base-repo secrets to a fork's code. Every outside contribution here arrives from a fork, so that trade is not available. The comment at the guard says so, since the next person to notice the skipped runs will reach for it. In-repo branches, Renovate's included, still run. Also corrects a stale note in CLAUDE.md: the `ajv` entry still described `ModdedBlueprintError` and `TrainBlueprintError` as declared-but-never-thrown, and #262 deleted both. The point it was making survives - ajv is ~100 kB and nothing branches on its result - so the entry keeps that and records what went. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018N3pm7fQQDv6HTVz1TEpmE * Correct CLAUDE.md's vite-plus entries against what the repo actually pins Three corrections, each measured rather than read off the file. The documented local-install command did not set the version at all. It read `VP_VERSION=0.2.8 VP_NODE_MANAGER=yes curl -fsSL https://vite.plus | bash`, and an assignment ahead of a command applies to that command alone - `curl` got the variables and the `bash` on the far side of the pipe read an empty string. Measured against a stub script, which printed `VP_VERSION=[]`. The installer then falls back to `VP_VERSION="${VP_VERSION:-latest}"`, read off the script itself, so anyone following that line installed `latest` rather than the pin. That is the "green, and wrong" split the same file warns about one section down, with a local toolchain silently different from the lockfile's and CI's. The command now downloads the script and runs it with the variables ahead of `bash`, matching setup-vp/action.yml, and sets VP_HOME for the layout reason #260 established. Syntax-checked with `fish -n`, since it is a fish block. The pin is 0.2.9 everywhere in the repo - root, editor and website package.json, the root overrides alias, and VP_VERSION in setup-vp/action.yml - while the file still said 0.2.8 in three places. It also claimed 0.2.8 was `latest` as of 2026-08-11; `npm view vite-plus dist-tags` gives 0.3.0 today. That entry has now gone stale twice, which is its own best argument, so it says so and points at the command to re-measure with. And the installer-checksum note said the hash did not move across 0.2.6 -> 0.2.8, so a bump usually leaves it alone. True when written, and it is the reassurance that made 2026-08-24 expensive: the script rotated with VP_VERSION untouched and every job on every branch failed at `Set up Vite+`. The note now records that a hash can move with no bump at all. Re-fetched today and the current sha256 still matches the pin, so nothing in CI needs changing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018N3pm7fQQDv6HTVz1TEpmE --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Measures what the blueprint GUI's "Grid position" field does to an exported blueprint, on Factorio 2.0.77 - the version this editor targets. It is the first probe here to run on the shared
factorio-oracleCLI (#235), and the first to need a person at the keyboard since the zoom one.Why it had to be interactive
Nothing in
runtime-api.jsonreaches this field. The game exposes three blueprint snapping attributes and none of them is it, andcreate_blueprinttakes no anchor parameter. The field writes no key at all into the export; it translates the entity and tile coordinates instead. So there is no script that can set it and no dump that can read it back.What it found
survivingReadings: ["entityEdgesAndTiles"], with all three controls passing and the three rival readings killed on every scored row.The rule is
-floor(min corner) = T, the corner taken over entity edges and tiles. It is an absolute target, not a relative nudge: setting 8,9 on top of 3,5 moved the corner to -8,-9 rather than -11,-14, which is the only thing separating those two readings and the reason the session asks for a second value on top of a first.The apparent contradiction was a name collision
tools/oracle/fixtures/blueprint-grid-position.jsonscores the "grid position moves entities" premise 0 of 2 on this same 2.0.77, and PR #243 rebuildsgetGridPositionDisplay()on the opposite premise. Both are right. The panel carries three X/Y pairs:The older fixture measured the Absolute row. Two of the three pairs were on screen at once holding different values, which is as direct as that distinction gets. The new fixture carries a
supersedesblock saying the two answer different questions, andfactorio-oracle'sdocs/method.mdhas been corrected: its PR #222 entry read "a blueprint's grid position moves its entities. It does not", which was true of the attribute and read as a refutation of the field.One finding for PR #243
getGridPositionDisplay()builds its minimum frome.position.x, the entity centre. The game takes the edge, so the formula is off byfloor(size/2)on whichever axis a multi-tile entity sets the minimum. A belt-edged blueprint agrees; an assembler-edged one is a tile out.What hides it is that the editor stays self-consistent:
commitGridPositionsolves for the offset that makesgetGridPositionDisplay()read the target back, using the same formula on both sides, so the box always shows what was typed. Only a comparison against the game exposes it. This is posted on #243 as finding 15.The measurement cannot say whether the game uses the tile footprint edge or the collision box edge, and that decides the fix: for
assembling-machine-1they are 9.0 and about 9.1, which floor to the same integer. Separating them needs an entity whose collision box is inset by more than the fractional part.Also here
tools/oracle/fixturesis exempt from oxfmt. The generator writesJSON.stringify(x, null, 4)and oxfmt collapses short arrays, so whichever ran last won and the other reported a failure. "Never hand-edit a fixture to make something pass" applies to a formatter too.stepsNotCaptured. An interactive probe ends when the person stops playing, and a fixture that simply omits a step reads as "this step does not exist" rather than "it was not run". This run has["abs-2-6", "snap-off"]; neither is load-bearing.SESSION.md's run command carries acd.control_lua_filein aprobe.jsonis repo-relative and the CLI reads it against the shell's cwd, so an absolute--probefrom elsewhere fails naming the Lua file rather than the cause.SESSION.mdstep 6 said to pick Absolute, which is already the default the moment Snap to grid is ticked, and described its pair as the Grid position one.No production code changes.
vp checkis clean.🤖 Generated with Claude Code
https://claude.ai/code/session_019RiP5JRvEpzDXM4AY5ouGN