algorithmic-art: wrapAround() in generator_template.js never wraps — returns out-of-range values and discards the overshoot

Summary

wrapAround() in skills/algorithmic-art/templates/generator_template.js:154-159 does not wrap. It snaps every negative input to max and every input above max to 0, so it loses the offset it is supposed to preserve — and max is not even a legal result for the range it claims to constrain into.

This is not a "maybe they meant clamp" case. Measured over max = 800 and the inputs -2400..2400, the shipped body agrees with a wrap on 802 inputs, with a clamp on 801, and with either reading on 803 total — 16.7%. The other 83.3% agree with neither. That union is almost entirely the identity band: the only inputs outside [0, max] on which it happens to return what a wrap would are 1600 and 2400, both because the overshoot collapses to 0. In other words, every branch that exists to handle an out-of-range value is wrong, and it is wrong under either intent.

Location

skills/algorithmic-art/templates/generator_template.js, on main @ 3337550:

154  // Constrain to bounds
155  function wrapAround(value, max) {
156      if (value < 0) return max;
157      if (value > max) return 0;
158      return value;
159  }

Why it matters here

SKILL.md makes these files the literal foundation of every artifact this skill produces, and this function is complete, copy-ready code — unlike the // fill this in stubs elsewhere in the same file:

• SKILL.md:396 — generator_template.js is "Reference for p5.js best practices".
• SKILL.md:399 — "Embed algorithms inline in the HTML artifact", i.e. these utilities are pasted into the shipped artwork.
• SKILL.md:105-127 — the template is the "LITERAL STARTING POINT", "Keep all FIXED sections exactly as shown".

A best-practice utility that silently returns a wrong coordinate is copied into whatever is built on top of it, and it never throws, so nothing surfaces the mistake.

Reproduction

The file is only declarations and its top level makes no p5 calls, so the shipped function can be evaluated in bare node — no browser, no p5 needed:

// node repro.js   (from the repo root)
const fs = require("fs"), vm = require("vm");
const ctx = { console }; vm.createContext(ctx);
vm.runInContext(fs.readFileSync("skills/algorithmic-art/templates/generator_template.js", "utf8"), ctx);

const wrap  = (v, m) => ((v % m) + m) % m;   // a real wrap
const clamp = (v, m) => Math.max(0, Math.min(m, v));

for (const v of [-1, -100, -801, 800, 801, 1601])
    console.log(`wrapAround(${v},800)=${ctx.wrapAround(v, 800)}  wrap=${wrap(v, 800)}  clamp=${clamp(v, 800)}`);

Measured output (node v24.18.0):

wrapAround(-1,800)=800  wrap=799  clamp=0
wrapAround(-100,800)=800  wrap=700  clamp=0
wrapAround(-801,800)=800  wrap=799  clamp=0
wrapAround(800,800)=800  wrap=0  clamp=800
wrapAround(801,800)=0  wrap=1  clamp=800
wrapAround(1601,800)=0  wrap=1  clamp=800

Full sweep, max = 800:

Two concrete consequences:

1. max is returned as an in-range coordinate. For a canvas created as createCanvas(800, 800) the valid x range is 0..799, and for the flow-field grid the template itself sets up (cols = floor(width / scl)) the valid index range is likewise 0..cols-1. wrapAround(-1, 80) yields 80, and flowField[80] is undefined — the wrapped index is still out of bounds, so the guard passes the value through and the failure moves to the next property access.
2. The overshoot amount is discarded. A particle one pixel past the right edge and one a thousand pixels past it both land on exactly 0; every negative lands on exactly max. Any position-dependent variation is erased.

Suggested fix

One line, and it is the idiom that handles both signs and any number of periods:

// Wrap into [0, max) so coordinates and indices stay valid for any overshoot
function wrapAround(value, max) {
    return ((value % max) + max) % max;
}

Same sweep after this change: 0 values differ from a wrap, 0 values land outside [0, 800), (800, 1600] maps to 800 distinct outputs and [-1600, -1] to 800 — the offset is preserved. The neighbouring utilities in the file (easeInOutCubic, mapRange, hexToRgb) were re-checked in the same harness as a control and are correct, so the harness is not what changed.

The comment above the function ("Constrain to bounds") is worth updating in the same pass, since describing a wrap as a constraint is plausibly how the branch logic went wrong.

Not proposed here, disclosed for completeness

colorFromPalette(index) at :141-143 uses index % params.colorPalette.length without the sign correction, and JS % keeps the sign of the dividend. Measured with a 3-entry palette:

colorFromPalette( 0) = #d97757     colorFromPalette( 3) = #d97757
colorFromPalette(-1) = undefined   colorFromPalette(-3) = #d97757   (−3 % 3 is −0)
colorFromPalette(-4) = undefined

Positive overshoot is handled; only negative indices fall through to undefined. I deliberately left it out of the fix above rather than widening it — wrapAround is the function whose stated job is range-normalisation, and a one-function change is independently verifiable. Happy to send it separately if maintainers want both.