fix(trading): a 0 bps slippage suggestion is not a falsy value

[gomesalexandre]

Found by inspection, no filed issue.

SlippageToleranceResponse.slippageBps is typed number | null, with null as
the explicit "no suggestion" sentinel, distinct from a legitimate 0. Three call
sites in packages/trading/ checked this with plain truthiness instead of a
null-check, so a partner explicitly supplying slippageBps: 0 via the public
advancedSettings.getSlippageSuggestion extension point (e.g. their own model
concluding a highly-correlated pair like USDC/USDT needs zero slippage buffer) got
silently discarded and replaced with the nonzero default:

• resolveSlippageSuggestion.ts:55 fell through to defaultSuggestion instead of
  routing 0 through suggestSlippageBps().
• getQuote.ts:165 corrupted the suggestedSlippageBps metadata field returned to
  callers/UIs (a UI showing "AUTO: 0%" would instead show "AUTO: 0.5%").
• getQuote.ts:170 skipped the order-rebuild branch entirely, leaving the order
  built with a stale nonzero slippage instead of the partner's explicit 0.

Fix: swapped the truthy checks for != null / ?? at all three sites.

One precision worth calling out: honoring a 0 suggestion doesn't produce a
zero-slippage order outright. suggestSlippageBps() always adds a fee-derived
component on top of the volume component it's fed (suggestSlippageBps.ts:47-60)

• a 0 suggestion only zeroes the volume part, same treatment as any other
  suggested value. Final slippage is typically a few bps from the fee component
  alone, 0 only when the fee itself rounds to 0. That's the correct, consistent
  behavior for this input - just flagging it so the fix doesn't read like a
  straight 0-in/0-out passthrough.

Direction is safe either way

Checked getQuoteAmountsAfterSlippage.ts and getOrderToSign.ts: sell orders
subtract slippage from the min-receive amount, buy orders add it to the max-pay
amount. Honoring a smaller (or 0) suggestion means a tighter limit in both
directions - the user can never end up with a worse execution price than quoted,
only a higher chance of no-fill. Grepped the rest of the monorepo (order-book,
bridging, composable, examples) for anything relying on the old behavior - nothing
does; every current caller either passes an explicit slippageBps (bypassing the
AUTO/suggestion lane entirely) or forwards its own slippage value directly. The
only lane whose output changes is a partner callback that actually returns 0.

Testing

New test cases in resolveSlippageSuggestion.test.ts and getQuote.test.ts
covering the slippageBps === 0 suggestion at all three sites. Guard-validated
per-site: reverting each fix individually causes its corresponding new test to
fail with the exact predicted wrong value (falls to defaultSuggestion/50 BPS, or
the rebuild branch gets skipped so appData.metadata.quote.slippageBips stays at
the stale default instead of reflecting 0), restoring passes. Also fixed a
pre-existing misleading log line in the branch this change makes always-taken when
a suggestion exists (was unconditionally claiming the suggestion is "greater than"
the default, which is false at 0 and in general).

Full packages/trading suite: 256 passed / 2 skipped (pre-existing, unrelated) /
258 total. typecheck and lint both clean. prettier --check clean on all
changed files.

Summary by CodeRabbit

• Bug Fixes

  • Preserved an explicitly suggested slippage of 0 for AUTO-slippage quotes.
  • Prevented zero-value slippage suggestions from falling back to the default.
  • Ensured quote calculations and volume adjustments correctly handle zero slippage.

• Tests

  • Added regression coverage for zero-slippage quote and suggestion scenarios.

[coderabbitai]

Warning

Review limit reached

Next included review available in 56 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 64ef96a5-5f23-4515-8286-9a8ac2903bd4

📥 Commits

Reviewing files that changed from the base of the PR and between ac32db3 and 2a744da.

📒 Files selected for processing (4)

• packages/trading/src/getQuote.test.ts
• packages/trading/src/getQuote.ts
• packages/trading/src/resolveSlippageSuggestion.test.ts
• packages/trading/src/resolveSlippageSuggestion.ts

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8f47a69c-705f-4498-b2e9-13e2d759944a

📥 Commits

Reviewing files that changed from the base of the PR and between 8a756e8 and ac32db3.

📒 Files selected for processing (4)

• packages/trading/src/getQuote.test.ts
• packages/trading/src/getQuote.ts
• packages/trading/src/resolveSlippageSuggestion.test.ts
• packages/trading/src/resolveSlippageSuggestion.ts

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The change preserves an explicit slippage suggestion of 0. Slippage resolution, AUTO-slippage quote generation, app-data rebuilding, and regression tests now handle zero without applying the default suggestion.

Changes

Zero Slippage Preservation

Layer / File(s)	Summary
Preserve zero slippage suggestions packages/trading/src/resolveSlippageSuggestion.ts, packages/trading/src/resolveSlippageSuggestion.test.ts	Slippage resolution now treats 0 as a provided suggestion. Tests verify zero output and a zero volume multiplier.
Propagate zero slippage through quotes packages/trading/src/getQuote.ts, packages/trading/src/getQuote.test.ts	AUTO-slippage quote generation preserves 0 and rebuilds app data with zero slippage. Tests cover the returned suggestion and app-data value.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to ac32d

The change preserves explicit 0 bps slippage suggestions instead of replacing them with the default, while retaining the existing fee-derived slippage behavior. No actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main fix: preserving legitimate 0 bps slippage suggestions instead of treating them as falsy.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[azebuado]

Thanks for the suggestion @gomesalexandre
Can you add a bit more info on why this change is needed exactly? What's your use case for needing the 0 suggestion to be piped through?

[gomesalexandre]

Fair ask — the use case is a partner using advancedSettings.getSlippageSuggestion (the public extension point for AUTO-slippage) to plug in their own model, e.g. concluding a highly-correlated pair like USDC/USDT genuinely needs zero volume-driven slippage buffer. slippageBps: 0 from that callback is a legitimate "I have a suggestion, and it's zero" answer — distinct from null, which is the explicit "no suggestion" sentinel. The three call sites used plain truthiness (slippageBps &&) instead of a null-check, so that legitimate 0 got silently discarded and replaced with the default 50bps instead.

To be precise about the effect (also noted in the PR description): this doesn't produce a zero-slippage order outright — suggestSlippageBps() always adds a fee-derived component on top of whatever volume component it's fed, so honoring a 0 suggestion only zeroes the volume part. Final slippage is typically still a few bps from the fee component alone.

Checked direction-safety too: sell orders subtract slippage from min-receive, buy orders add it to max-pay, so honoring a smaller/zero suggestion only ever tightens the limit in both directions — never a worse execution price, only a marginally higher chance of no-fill. Grepped the rest of the monorepo for anything relying on the old behavior — nothing does; every other caller either passes an explicit slippageBps directly (bypassing this lane) or forwards its own value. The only thing that changes is what a partner's own 0 callback actually does.

Also pushed a commit refreshing an unrelated stale live-API snapshot in packages/composable that was failing the test check — that test hits the live programmatic-orders API with no mocking and hadn't been refreshed since it was added, unrelated to this change but blocking CI here.

[gomesalexandre]

Rebased onto latest main to resolve the conflict. Dropped the earlier stale-snapshot-refresh commit — main since split that live-API test out into ProgrammaticOrderApi.int.ts (#978), so the unmocked snapshot no longer exists in ProgrammaticOrderApi.spec.ts and the CI-blocking issue it patched is already resolved upstream by a different route. Down to the two slippage-fix commits now; full suite green after the rebase.