feat(retry-strategies): make base parameter optional and defaults to 0

[vabatta]

Changes the strategies using a base to be optional and defaults to 0:

• ExponentialBackoff / exponential()
• FibonacciBackoff / fibonacci()
• FullJitterBackoff / fullJitter()
• EqualJitterBackoff / equalJitter()
• DecorrelatedJitterBackoff / decorrelatedJitter()

[claude]

Code Review for PR #15

Summary

This PR makes the base parameter optional (defaulting to 0) for five backoff strategies. The change is backward compatible and the implementation is clean.

---

✅ Strengths

1. Backward Compatibility: Making base optional with a default value doesn't break existing code - all current usages still work
2. Consistent Implementation: The change is applied uniformly across all five strategies
3. Test Coverage: New tests verify the default behavior for each strategy
4. Documentation Updated: README correctly reflects the new optional parameter
5. Type Safety: TypeScript signatures properly reflect the optional parameter

---

⚠️ Concerns & Questions

1. Questionable Default Value

The default of base = 0 produces zero-delay retries, which is rarely useful in practice:

const strategy = exponential(); // base=0, cap=Infinity
strategy.nextBackoff(); // 0ms
strategy.nextBackoff(); // 0ms (0 * 2^1 = 0)
strategy.nextBackoff(); // 0ms (0 * 2^2 = 0)

Impact: This could lead to:

• Immediate retry storms if users forget to specify base
• Thundering herd problems when services are under load
• Resource exhaustion from tight retry loops

Recommendation: Consider one of these alternatives:

• Don't make base optional - requiring an explicit value forces users to think about retry timing
• Use a more sensible default like 100 (100ms) that provides some breathing room
• Document prominently that base=0 should only be used intentionally for testing

2. API Design Philosophy

Why make base optional but not other strategies' required parameters?

• ConstantBackoff still requires delay (packages/retry-strategies/src/backoff/constant-backoff.ts:15)
• LinearBackoff still requires increment (and others)

Question: What's the use case driving this change? If it's for convenience, why only these five strategies?

3. Edge Case: cap < base Validation

With base = 0 as default, the validation logic at exponential-backoff.ts:34 now allows:

exponential(undefined, 50); // base=0, cap=50 ✓
exponential(100, 50);        // base=100, cap=50 ✗ RangeError

While technically correct, this inconsistency might confuse users. Consider if explicit validation or better error messages would help.

---

🔍 Code Quality

No issues found:

• ✅ Proper parameter validation maintained
• ✅ No security concerns
• ✅ No performance regressions
• ✅ Follows repository conventions (BDD tests, proper imports)
• ✅ Test assertions use ctx.plan() correctly

---

📋 Suggestions

1. Add usage warnings in JSDoc:

   /**
    * @param base - Base delay in milliseconds (>= 0, default: 0)
    *               WARNING: Using default (0) will result in no delay between retries
    */

2. Consider integration test showing practical use case for base=0 default

3. Verify impact on existing retry patterns - does this change encourage antipatterns?

---

🎯 Recommendation

Conditional Approval - The code quality is excellent, but I recommend addressing the API design concerns before merging:

1. Clarify the use case for base=0 default
2. Consider whether a different default (like 100ms) would be more appropriate
3. If keeping base=0, add prominent documentation warnings

Otherwise, the implementation is solid and well-tested. Great work on maintaining consistency across all affected strategies! 👍

[vabatta]

Actually, doesn't seem to make sense for now.