API hardening & cleanup (ROAST.md architectural review)

[MasterOfBinary]

Summary

Tracker for the API/code-quality points raised in an architectural review (the abandoned ROAST.md from PR #58). Each point is annotated with its current status on master (66070eb) so the still-live items are actionable and the already-fixed ones are recorded.

Tone of the original was deliberately harsh ("a grumpy senior staff engineer"); the substance is summarized neutrally below.

Status of each point

#	Critique	Status
1	Item.Data was interface{} everywhere → no compile-time type safety	✅ Fixed — generics migration (#60); master is Batch[T], New[T any].
2a	A whole goroutine + buffered channel (doIDGenerator, b.ids, IDBufferSize) just to increment a counter	⏳ In progress — open PR #64 replaces it with an inline sync/atomic counter and removes the IDBufferSize API.
2b	ExecuteBatches locks a mutex per error (batch/helpers.go:119)	⚠️ Still live — collect errors in a local slice per goroutine and append once.
3	IgnoreErrors spawns a goroutine that leaks if the channel never closes; errors.go hand-rolls Unwrap()	🟡 Partial — master now nil-guards the channel (avoids the nil-channel leak); the "never-closed channel" case is caller-owned. Revisit Unwrap vs %w if desired.
4	fixConfig silently mutates input (MinItems: 0 → 1, batch/batch.go:453)	⚠️ Still live — prefer validating and returning an error (or a documented default) over silent rewrite. Tracked jointly with the resource-limits + config-validation issue.
5	batch.Batch stutters; core API could live in root gobatch so callers write gobatch.New()	🟡 Design decision — would be a breaking rename; reasonable for a v0 but needs a deliberate call.
6a	New returns *Batch (concrete) rather than an interface, hurting mockability	🟡 Design decision — master: func New[T any](config Config) *Batch[T].
6b	Library panics instead of erroring: WithBufferConfig after Go() (batch.go:113) and concurrent Go() (batch.go:240)	⚠️ Still live — APIs should be hard to misuse, not booby-trapped: return errors, or make buffer config immutable at construction (functional options).

Actionable items (still live)

• 2b — Stop locking per-error in ExecuteBatches; accumulate locally, merge once.
• 4 — Validate config instead of silent mutation (with the resource-limits/validation work).
• 6b — Replace library panics with returned errors and/or immutable-at-construction config.
• (optional) 3 — Reconsider IgnoreErrors contract / use fmt.Errorf("%w", …).
• (optional) 5 / 6a — Decide on package/type naming and concrete-vs-interface return as part of the path to v1.

Source (for recovery — branch is being deleted)

roast-codebase-7386035102356312858 @ PR #58 (ROAST.md). PR is closed but its diff remains viewable.

---

Related: #73 (resource limits + config validation — point #4 lands there)

[MasterOfBinary]

Status update vs master @ 58cee73 (the annotations above predate the #64 merge):

• 2a DONE — feat(batch)!: single-use Batch + Go returns (<-chan error, error); inline item IDs #64 merged: inline item IDs, ID goroutine and IDBufferSize removed.
• 6b HALF DONE — second/concurrent Go() now returns ErrBatchUsed (feat(batch)!: single-use Batch + Go returns (<-chan error, error); inline item IDs #64); WithBufferConfig after Go() still panics (batch/batch.go:125). Note PR feat(batch): configurable context-cancellation mode (WithCancelMode) #76 adds a same-pattern WithCancelMode panic — if 6b is resolved, both should convert together.
• 4 lands via Resource limits + configuration validation #73 (validation); PR docs: clarify config zero-value and clamp semantics #67 documents the clamp semantics in the meantime.
• 2b still live and only tracked here: ExecuteBatches locks the mutex per error in batch/helpers.go — accumulate locally, merge once under one lock.
  Remaining actionable: 2b, 6b remainder. Optional: 3 (Unwrap is idiomatic Go — suggest dropping), 5/6a (v1 naming/interface decisions). ROAST.md itself was never committed (recovered from closed PR Add architectural critique (ROAST.md) #58), which is fine — this issue is its tracker.