[bug] slice with an unparseable limit silently drops every record, and the run still reports success

[serge-ivo]

What happens

lib/steps.ts:867:

const limit = input.limit === undefined || input.limit === null
    ? items.length                                        // absent → keep ALL
    : Math.max(0, Math.floor(Number(input.limit) || 0));  // unparseable → keep NONE

Number(x) || 0 turns anything non-numeric into 0, and slice treats 0 as "keep zero
records". So:

limit	records kept
absent / null	all (documented default)
50 / "50"	50
""	0
"fifty"	0
false	0

An unparseable limit produces the single most destructive outcome available, while an absent
one keeps everything. That asymmetry is the bug.

Why it is reachable

Step inputs are not validated against jsonSchema before the handler runs — executePipelineStep
calls resolveInputs then runRegistryTool (lib/pipeline.ts:288-289, 296-297), and
runRegistryTool performs no schema check. The declared limit: { type: "number" } is a hint to
the model, not a gate.

So an empty string reaches the handler whenever a $param resolves to blank — a trigger or
connection config with the limit field left empty, a config.params entry that isn't set, a
hand-written pipeline JSON with "limit": "".

Why it is bad

slice exists to sit in front of per-item connector calls — its own description: "cap a list
before a per-item connector call (enrich / forEach) so a long source can't turn into unbounded
spend." So it is placed mid-chain, exactly where emptying the list is invisible:

• Downstream enrich / forEach / dedupe_upsert run over nothing.
• Every step succeeds. The run completes, not fails.
• The result is "0 leads", indistinguishable from "no businesses matched your search".

Nobody debugs a successful run that found nothing. The evidence is technically present — the step
returns dropped: items.length - out.length — but nothing surfaces it.

The rest of the file gets this right

Every other numeric input in steps.ts falls back to something non-destructive:

concurrency  Number(...) || 4        maxPages   Math.min(Number(...) || 5, 50)
extentKm     Number(...) || 1        timeoutMs  Math.min(Number(...) || 12000, 30000)
offset       Number(...) || 0        maxTokens  Number.isFinite(...) ? ... : 500

maxTokens even uses Number.isFinite, which is the correct test. slice.limit is the only one
whose fallback destroys data.

Secondary, milder instance: flatten's depth (:494) has the same shape — a bad value
becomes 0, silently no-opping the flatten rather than defaulting to the documented 1.

Suggested shape

• Use Number.isFinite (as maxTokens does). A non-finite limit should either keep all —
  matching the absent-value default — or fail the step with a clear message. Failing is
  arguably better: a limit you asked for and didn't get is worth stopping over.
• Same for flatten.depth (fall back to 1, not 0).
• Consider validating step inputs against the declared jsonSchema in runRegistryTool, which
  would close this class rather than these two instances. Larger change; worth its own discussion.

Verification

• limit: "", "abc", false → keeps all (or fails), never silently zero.
• limit: 0 explicitly → still keeps zero (that is a real request).
• A pipeline whose $param limit is blank produces the same result as one with no limit at all.

[serge-ivo]

Shipped in fa7b00e.

Verified: true. steps.ts:867 was exactly as quoted, and the asymmetry is real — absent kept all, unparseable kept none. The secondary flatten.depth instance (:494) was there too.

What shipped

• lib/step-number.ts — parseStepNumber(raw) returns absent | {value} | {invalid, reason}. Deliberately stricter than Number.isFinite(Number(x)), which still maps "", false and [] to 0. Only undefined/null are absent. Plus stepNumberError for the message.
• slice.limit, slice.offset and flatten.depth now fail the step on an unreadable value rather than defaulting.

I went with failing over falling back, on your own reasoning — a limit you asked for and didn't get is worth stopping over — and because a soft fallback would still be invisible mid-chain. The message names the field and points at the reference feeding it, since the fault is nearly always a blank $param off a settings or trigger field.

Note on flatten.depth: you suggested falling back to 1. I failed it instead, for consistency and because a no-opped flatten passes arrays-of-arrays to a map/filter that then succeeds over the wrong shape — same silent class. Say the word if you'd rather it defaulted.

Explicit limit: 0 and depth: 0 still work — those are real requests, and there are tests holding that line. Numeric strings ("2") still work, since that's the normal shape of a $param.

Left open: the broader suggestion — validating step inputs against the declared jsonSchema in runRegistryTool — is not done. It would close the class rather than these two instances, but it changes the contract of every registry tool at once (including the connector tools, whose handlers currently do their own coercion) and deserves its own issue. The other numeric fields you listed all have non-destructive fallbacks and were left alone.