Current outcome
This is a non-executable coordination issue for GoBatch's application-facing contracts. Keep focused issues authoritative and preserve the useful parts of the old review without treating every stylistic critique as required work.
Replanned on 2026-09-06 against GoBatch master 63ef757 and ShitQuant's recorder, enrichment, paper/replay, and flow workloads. This is an implementation target, not a claim that the behavior already exists.
Required behavior
Verification and completion
Close only when the committed child outcomes and release evidence for the chosen tranche are complete; deferred ideas do not become implementation obligations. Each child owns its actual code checks.
Follow the repository's formatting, race-test, vet/lint, package documentation, example and changelog requirements for the changed surface. Report the actual supported behavior and migration; do not treat a passing coverage percentage as proof of these outcomes.
Scope and relationships
Preserves the existing review tracker and comment history. Focused follow-ons: #95 lifecycle, #96 bounded-core release, #97 flow delivery epic, #98 flow runtime, #99 standalone flow release and #100 optional request/flow composition. No root-package rename or OnyxCore dependency.
Design history
The earlier report/prototype remains below for provenance; the requirements above supersede conflicting prescriptions. Existing discussion is preserved.
Original issue: API hardening & cleanup (ROAST.md architectural review)
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)
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)
Current outcome
This is a non-executable coordination issue for GoBatch's application-facing contracts. Keep focused issues authoritative and preserve the useful parts of the old review without treating every stylistic critique as required work.
Replanned on 2026-09-06 against GoBatch master
63ef757and ShitQuant's recorder, enrichment, paper/replay, and flow workloads. This is an implementation target, not a claim that the behavior already exists.Required behavior
Verification and completion
Close only when the committed child outcomes and release evidence for the chosen tranche are complete; deferred ideas do not become implementation obligations. Each child owns its actual code checks.
Follow the repository's formatting, race-test, vet/lint, package documentation, example and changelog requirements for the changed surface. Report the actual supported behavior and migration; do not treat a passing coverage percentage as proof of these outcomes.
Scope and relationships
Preserves the existing review tracker and comment history. Focused follow-ons: #95 lifecycle, #96 bounded-core release, #97 flow delivery epic, #98 flow runtime, #99 standalone flow release and #100 optional request/flow composition. No root-package rename or OnyxCore dependency.
Design history
The earlier report/prototype remains below for provenance; the requirements above supersede conflicting prescriptions. Existing discussion is preserved.
Original issue: API hardening & cleanup (ROAST.md architectural review)
Summary
Tracker for the API/code-quality points raised in an architectural review (the abandoned
ROAST.mdfrom PR #58). Each point is annotated with its current status onmaster(66070eb) so the still-live items are actionable and the already-fixed ones are recorded.Status of each point
Item.Datawasinterface{}everywhere → no compile-time type safetymasterisBatch[T],New[T any].doIDGenerator,b.ids,IDBufferSize) just to increment a countersync/atomiccounter and removes theIDBufferSizeAPI.ExecuteBatcheslocks a mutex per error (batch/helpers.go:119)IgnoreErrorsspawns a goroutine that leaks if the channel never closes;errors.gohand-rollsUnwrap()masternow nil-guards the channel (avoids the nil-channel leak); the "never-closed channel" case is caller-owned. RevisitUnwrapvs%wif desired.fixConfigsilently mutates input (MinItems: 0→1,batch/batch.go:453)batch.Batchstutters; core API could live in rootgobatchso callers writegobatch.New()Newreturns*Batch(concrete) rather than an interface, hurting mockabilitymaster:func New[T any](config Config) *Batch[T].WithBufferConfigafterGo()(batch.go:113) and concurrentGo()(batch.go:240)Actionable items (still live)
ExecuteBatches; accumulate locally, merge once.panics with returned errors and/or immutable-at-construction config.IgnoreErrorscontract / usefmt.Errorf("%w", …).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)