Let the loose bin pick its layer, and add one, without leaving the page

[Oliver-Johnson]

Closes #4.

Design a bin on its own, size it past what the drawer has room for, and the landing check refuses it:

no free 3×4 space on layer 1 — clear some cells, or add a layer

Both remedies it names lived inside #s-layout, which body.binfocus hides wholesale in one CSS rule. So the only way to take either was to Discard the bin, go back to the drawer, fix it there, and start the bin again. The message told you what to do and hid the controls for doing it.

The change

A layer picker and an Add a layer on top button now sit in the focus bar, shown for a loose bin only. What shows and hides them is the CSS list under body.binscratch — not JS assignments — per the handover's rule that readControls() already makes ~25 show/hide decisions and a restore made of 25 more eventually half-restores.

Switching layers banks no undo entry: it changes where the bin would go, and the drawer's own layer tabs don't file one for the same choice either.

The trap this had in it

snapshot() in scratch mode captures only the loose bin — nothing about layers or cur — and pushUndo() files to the loose bin's stack. Adding a layer edits the drawer. Routing it through pushUndo() would bank an entry that restores the bin, leaves the new layer standing, and spends an Undo that appears to do nothing.

It now goes on the drawer's stack, where the change will actually be seen. pushOn holds the single implementation and pushUndo / pushDrawerUndo name their target — written once rather than twice, since the handover records that jointKind cost three bugs by being copied four times.

A bug the tests caught in my own work

I first called refresh() alone. The landing reason is written by applyFocus(), which runs at the tail of readControls() — so the message kept answering the layer it was written for rather than the one just chosen. Both handlers now run the full readControls / drawLayerTabs / drawMap / refresh sequence every other mutating handler uses.

Worth knowing: support is judged per bin, not per layer

Correction to an earlier version of this description, which claimed an upper layer needs continuous support beneath it and that a half-empty layer below would refuse a bin just as firmly. That was wrong. Oliver flagged it and the code agrees with him.

seat(b, k) walks only the new bin's own footprint:

function seat(b, k) {
  const s = support(k);
  let z = null, flat = true, solidBelow = true;
  for (let dy = 0; dy < b.v; dy++)
    for (let dx = 0; dx < b.u; dx++) {
      const y = b.y + dy, x = b.x + dx;
      if (!s.ok[y] || !s.ok[y][x]) { solidBelow = false; continue; }

The rest of the layer is never consulted. support(k) marks ok[y][x] = false only where a lower layer is empty at that coordinate — so what is required is an unbroken, level column of bins under the new bin's own cells, and nothing at all is asked of the rest of the drawer.

Measured: with layer 1 holding one 2×2 bin out of 63 cells, a 2×2 placed on layer 2 directly over it returns solidBelow: true, flat: true, z: 21. A 3×3 over the same bin returns solidBelow: false — the footprint outgrew its support. A 2×2 over bare plate is likewise refused. The empty remainder of the layer is irrelevant either way.

So the refusal message is fine, and I withdraw the concern that it misleads. It only fires when layer N has no free spot for that footprint, which generally means the layer is densely occupied — precisely the condition that makes the layer above well supported.

The third case still uses full coverage, because that guarantees a landing spot exists wherever the loose bin ends up — not because a partial layer would refuse one. Commit 034a60f corrects the comment that said otherwise.

Verification

Four new cases in test/ui/scratch-layers.spec.js, each proven to bite:

ablation	result
pushDrawerUndo() → pushUndo() (wrong stack)	only the undo-target case fails
drop readControls() from the layer-change handler	only the re-answer case fails
both in place	all 4 pass

Each ablation kills exactly one case, and a different one — so neither case is redundant.

• Browser suite: 170 passed, 0 failed at --workers=1 (166 previously + 4 new).
• Headless suite all green; build.js --check clean. The id audit rose 114→116 referenced and the display-reachability audit passes, so the new controls are genuinely reachable.
• Built pages regenerated with node build.js, never hand-edited. style.css is embedded in every page, so all five outputs change.

Note on merge order

This will conflict with #7 in index.html — both regenerate built output. The resolution is to rebase and re-run node build.js, not to hand-merge generated files. Happy to do that once #7 lands.

Scope

Deliberately scratch mode only. In focus mode the bin already has a layer, so a picker there would mean moving a placed bin between layers — which needs its own fit and support checks and is a larger feature than the one asked for.

🤖 Generated with Claude Code

[Oliver-Johnson]

Verdict: one confirmed bug, otherwise good. (Comment rather than "request changes" only because GitHub refuses that event on a PR the authenticated account owns.)

Independent review — I did not write any of this.

This is the best-argued of the four PRs and the reasoning about the two undo stacks is exactly right. pushOn holding the single implementation, pushDrawerUndo naming its target, and routing the layer addition onto the drawer's stack are all correct, and I confirmed the trap is real rather than hypothetical. The scope call (scratch only, not focus) is the right one.

Both ablations reproduce exactly as claimed

I ran them rather than taking the table on trust. Copy-aside and copy-back, per the handover's warning about git checkout <file>.

ablation	result
pushDrawerUndo() → pushUndo()	1 failed (banks its undo on the drawer), 3 passed
drop readControls() from the layer-change handler	1 failed (re-answers the landing question), 3 passed
neither	4 passed

Each kills exactly one case and a different one. These tests bite.

The bug: the layer picker goes stale and says a full layer is empty

drawScratchLayers() caches on layers.length + '/' + cur, but the option text carries L.bins.length. Bin counts change without either key component changing, and startScratch() never resets scratchLayerKey. So:

1. Open the page, enter the loose bin → picker caches "1/0", shows Layer 1 · empty.
2. Discard, drag a bin onto layer 1.
3. Enter the loose bin again → key is still "1/0", so drawScratchLayers() returns early.

Measured in the browser:

OPTIONS on first entry : ["Layer 1 · empty"]
bins now on layer 1     : 1
OPTIONS on second entry: ["Layer 1 · empty"]     <-- wrong

The count is the only thing the picker adds over a bare layer number, and how full a layer is is precisely what governs whether the bin can land — so the one piece of information worth having is the one that goes stale. The refusal text underneath stays correct, which makes the two disagree on screen.

A key that carries the counts fixes it and subsumes the length:

const key = cur + '/' + layers.map((L) => L.bins.length).join(',');

Worth a fifth case, since nothing currently covers re-entry.

Smaller things

• The two scratchLayerKey = '' lines are dead. Both handlers change cur (and one changes layers.length), so the key differs on the next call regardless. Harmless, but they read as uncertainty about whether the cache is right — and with the count-carrying key above they become genuinely redundant.
• pushDrawerUndo re-spells snapshot()'s drawer branch. JSON.stringify({ layers, cur }) now appears twice, four lines apart. The PR body cites jointKind costing three bugs by being copied four times, and then makes a copy — small, but a drawerSnap() used by both would honour the rule the body invokes.
• Switching the picker mutates cur, which is drawer state, with no undo entry and no announcement. Discard after picking layer 3 and you land back in the drawer on layer 3 rather than where you left. Defensible (it is where the bin was going), but it is a silent side effect on the other document — worth a sentence in the focus bar or in the commit message.
• Comment style. The new comments use ASCII -- where this codebase uses em dashes: src/bins/ui.js on main has 142 — and zero --; src/bins/template.html has 50 and zero. Four new lines here break that. Trivial, but the prose voice in this repo is clearly deliberate.
• #scratchTarget label is 11px, under the 12px floor export.spec.js enforces at 375px. It passes today only because the audit runs outside scratch mode, and the neighbouring #scratchWhy is already 11px — so this follows precedent rather than setting one. Flagging it as pre-existing, not as this PR's doing.

Merge order

No longer a concern: this branch is already rebased past #7, and it auto-merges with #11, #9 and #10 with node build.js --check clean on the result. I checked by merging all four. The generated pages stay in sync, so no hand-merge is needed.

---

Reviewed independently; I am not the approver.

[Oliver-Johnson]

Confirmed stale-label bug. The key is layers.length + '/' + cur, but the option text below carries L.bins.length. A layer's bin count changes without either key component changing, and startScratch() does not reset scratchLayerKey — so re-entering the mode returns early on line 679 and leaves the old text standing.

Measured:

OPTIONS on first entry : ["Layer 1 · empty"]
bins now on layer 1     : 1
OPTIONS on second entry: ["Layer 1 · empty"]

Carry the counts and the length comes along for free:

const key = cur + '/' + layers.map((L) => L.bins.length).join(',');

That also makes the two scratchLayerKey = '' resets (lines 698, 705) honestly redundant rather than defensively redundant.

[Oliver-Johnson]

JSON.stringify({ layers, cur }) is now written twice — here and in snapshot() four lines up. The PR body cites jointKind costing three bugs by existing in four copies; this is the same shape of thing, one copy in.

const drawerSnap = () => JSON.stringify({ layers, cur });
const snapshot = () => (scratch ? JSON.stringify({ scratch }) : drawerSnap());
function pushDrawerUndo() { pushOn(undoStack, redoStack, drawerSnap()); }