feat(fx): consume the shared card-markup endpoint, delete both frontend copies - #2658
Merged
Conversation
…end copies The card-vs-Peanut markup was modeled twice in the frontend — once as a server action, once copied into the merchant page with a header saying to delete it when #2108 landed. #2108 landed. Both computed their own dolarapi call and their own issuer fee, and neither could agree with the rate on screen. The hook keeps its signature and its never-fails contract: a backend outage falls back to the same static table as before, so a frontend deploy that precedes the backend degrades to today's numbers instead of blanking a surface.
…compute Also splits the two failure kinds: a 404 is the backend proving there is no comparison to publish, so the row is hidden rather than filled with the static assumption, which exists for an unreachable backend.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe card-markup flow now uses a documented FX API, validated client utilities, and React Query fallback handling. Consumers distinguish unavailable rates from loading states. The PR adds the ChangesCard markup API migration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MerchantLandingPage
participant useCardMarkupRate
participant fetchCardMarkup
participant CardMarkupEndpoint
MerchantLandingPage->>useCardMarkupRate: request ARS markup
useCardMarkupRate->>fetchCardMarkup: fetch card markup
fetchCardMarkup->>CardMarkupEndpoint: GET /fx/card-markup
CardMarkupEndpoint-->>fetchCardMarkup: API response
fetchCardMarkup-->>useCardMarkupRate: live, static, or null result
useCardMarkupRate-->>MerchantLandingPage: consumer markup state
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Contributor
Code-analysis diffPainscore total: 7141.73 → 7137.94 (-3.79) 🆕 New findings (247)
…and 227 more. ✅ Resolved (242)
…and 222 more. 📈 Painscore deltas (top movers)
|
Contributor
Author
|
@coderabbitai review |
Contributor
|
Contributor
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
Review found the distinction collapsing at four points, each publishing a saving claim on evidence there is none: - a well-formed zero markup threw, so the hook fell back to the static 9.13% - an out-of-band locked-price recompute silently returned the market markup, still tagged live — the exact failure the recompute exists to prevent - the merchant page and the post-card-spend nudge both turned the hook's null into the static table, striking through every menu price by 9.13% fetchCardMarkup now returns null for 'the backend published no comparison' and throws only when the response could not be obtained or trusted. Also bounds the live observation age client-side, formats the CompareSavings date in UTC (a hydration mismatch on statically generated pages for any viewer west of Greenwich), rejects a negative percent instead of publishing it as a range, and formats amounts in the page locale.
abalinda
marked this pull request as ready for review
August 11, 2026 09:53
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/Marketing/mdx/CompareSavings.tsx`:
- Around line 55-56: Update the verifiedAt parsing logic in CompareSavings to
require exact YYYY-MM-DD input, reject invalid formats and calendar overflows
such as 2026-02-30, and verify the parsed UTC year, month, and day match the
input components before continuing. Add a regression test covering an overflow
date.
In `@src/utils/fx.utils.ts`:
- Around line 200-206: Update the zero check in the markup parsing flow around
positiveDecimal so every valid zero-valued decimal string, including “0.0” and
“0.00”, returns NONE before invalid or fallback handling; compare the parsed
numeric value with 0 rather than matching only the literal “0”. Add a regression
test covering alternate zero representations and their NONE result.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1a469522-a4b1-47af-aa1f-d2984a0c74e9
⛔ Files ignored due to path filters (1)
src/types/api.generated.tsis excluded by!**/*.generated.*
📒 Files selected for processing (15)
next.config.jssrc/app/actions/__tests__/card-comparison.test.tssrc/app/actions/card-comparison.tssrc/app/m/[slug]/MerchantLandingPage.tsxsrc/app/m/[slug]/card-comparison.tssrc/components/Marketing/mdx/CompareSavings.tsxsrc/components/Marketing/mdx/__tests__/CompareSavings.test.tsxsrc/components/Marketing/mdx/components.tsxsrc/components/TransactionDetails/provider-rows/LocalRailNudge.tsxsrc/constants/payment.consts.tssrc/hooks/__tests__/useCardMarkupRate.test.tsxsrc/hooks/useCardMarkupRate.tssrc/types/api.openapi.jsonsrc/utils/__tests__/fx.utils.test.tssrc/utils/fx.utils.ts
💤 Files with no reviewable changes (4)
- src/app/actions/tests/card-comparison.test.ts
- src/app/m/[slug]/card-comparison.ts
- next.config.js
- src/app/actions/card-comparison.ts
…dates
Both CodeRabbit findings are the same defect class as the last round — a
literal check that misses an equivalent representation and falls through to the
static claim.
The wire pattern admits "0.0" and "0.00", not just "0". Matching the text
sent those down the invalid path, so a backend saying 'no gap to show' produced
a 9.13% saving claim. The check now compares the parsed value.
new Date('2026-02-30') is 2 March, so a typo in a hand-authored MDX date would
publish a verification date that never happened. verifiedAt is now round-tripped
against an exact YYYY-MM-DD.
kushagrasarathe
left a comment
kushagrasarathe
left a comment
Contributor
There was a problem hiding this comment.
Agent review (advisory, on behalf of @kushagrasarathe) — full pass on the diff, no blockers. Two inline notes below, plus one repo-level note:
Back-merge: this PR targets main directly. After it lands, a main → dev back-merge is needed (same for peanut-api-ts#1322) or the branches drift.
Merge order stands as stated: peanut-api-ts#1322 must be merged and live on Render before this merges.
| return await fetchCardMarkup(code, mantecaPriceUsdToLocal) | ||
| } catch (error) { | ||
| // The backend has no comparison to publish — render nothing. | ||
| if (error instanceof FxApiError && error.status === 404) return null |
Contributor
There was a problem hiding this comment.
The PR body claims a UI-first deploy "degrades to today's static numbers" — it doesn't. An undeployed backend route returns Fastify's default 404, which is an FxApiError with status 404, so this line maps it to null and the comparison row is hidden, not served from the static table, until the backend ships.
Benign failure (a marketing row disappears, nothing breaks), and moot if #1322 merges first — but if you want the stated degradation to be true, distinguish the backend's deliberate 404 by its error body (error === 'FX_CARD_MARKUP_UNAVAILABLE') and only return null for that; let a route-missing 404 fall through to the static fallback.
| * ⚠️ The 100× trap: writing `"0.04"` for 4% publishes a claim a hundred | ||
| * times too small, and nothing downstream can tell the difference. | ||
| */ | ||
| markupPct: string |
Contributor
There was a problem hiding this comment.
Naming trap: markupPct here is in percent units ("4" = 4%), while the backend's /fx/card-markup field of the same name is a fraction ("0.04"). Both are loudly documented, but same name + 100× different units in the same PR pair is a foot-gun for the next author. Consider renaming this prop (e.g. feePercent) before content starts embedding it — cheap now, breaking later.
kushagrasarathe
approved these changes
Aug 11, 2026
kushagrasarathe
left a comment
kushagrasarathe
left a comment
Contributor
There was a problem hiding this comment.
Approved — merge only after peanut-api-ts#1322 is live on prod. Inline notes (404 fallback, markupPct units) are non-blocking.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Consumes the new
GET /fx/card-markupendpoint and deletes both frontend copies of the card-comparison model.The "how much more does a foreign card cost than Peanut" number was computed twice in this repo:
src/app/actions/card-comparison.ts— the qr-pay / LocalRailNudge lanesrc/app/m/[slug]/card-comparison.ts— a copy for the merchant pages, whose own header says "delete when feat(qr-pay): live card-vs-local-rail savings for ARS + BRL #2108 lands". feat(qr-pay): live card-vs-local-rail savings for ARS + BRL #2108 landed.Each had its own
ISSUER_FX_FEE = 0.03, its owndolarapi.comcall, and its own 9.13% fallback, and each fetched its own Peanut price — so neither could be guaranteed to agree with the rate rendered beside it. Both are gone. The model now lives in one place in the backend and is computed against the same market snapshot the displayed rate comes from, so the two agree by construction.Also adds
CompareSavings, an MDX component for the/compare/*pages that turns a dated, sourced competitor claim into a live savings sentence. It is registered but not yet used by any content — the content leg embeds it only after this ships to prod.Depends on peanutprotocol/peanut-api-ts#1322. Deploy the backend first.
Task
TASK-20265. Brief:
mono/ops/plans/2026-08-10-fx-card-markup-unification.md.Design notes / accepted trade-offs
404and "outage" are deliberately not the same failure. A404is the backend proving it has nothing to publish — an unmodeled currency, or live observations showing the rates have converged. Falling back to the static table there would advertise a 9% saving against evidence of none, so404renders no row. Any other failure means the model was unreachable, which is exactly what the static assumption is for. (This split came out of review; see the backend PR.)fetchCardMarkupre-derives the markup fromcomponentsagainst that locked rate. Otherwise the saving shown is not the saving the user gets.CARD_FX_MARKUP_BY_CURRENCYstays. It is both the client's last-resort fallback and the eligibility gatehasCardMarkupComparisonreads. Its doc comment now points at the backend.dolarapi.comis out of the CSPconnect-src(precedent:5fa013fccremoving frankfurter).git grep dolarapiis clean apart from one stale comment, now reworded.CompareSavingsprops are all string literals becausemdx-securityrejects JSX expression props.markupPctis in PERCENT units ("4"= 4%) — the 100× trap is documented loudly in the JSDoc, and an unparsable value degrades to a weaker dated sentence instead of publishing a wrong number./fx/card-markuppath was added tosrc/types/api.openapi.jsonrather than re-copying the whole spec — the committed snapshot already carries unrelated drift from the API repo, and pulling it in would bury this diff.Risks / breaking changes
I did itbutton on deposit USDC screen #1322 must be live before this. If it is not, every call 404s or fails and the hook serves the static table — which is exactly today's behaviour, so the failure mode is "no change", not "broken".git grepconfirms three importers, all updated in this PR.CompareSavingsships unused; no content references it yet. Adding an unknown MDX component to content would fail the SSG build, which is why the content leg is gated on this reaching prodmain.QA
pnpm prettier --check .clean ·npm run typecheckclean ·npm test226/226 suites, 2900 passing ·pnpm buildgreen (1045 static pages generated) ·npm run check:apigreen.fx.utils.test.ts(live contract, locked-price recompute, 9 rejection cases),useCardMarkupRate.test.tsx(served / static-fallback / 404-hides-row / disabled),CompareSavings.test.tsx(static lane, live lane, range upper bound, degraded props)./m/stainshould show the live rate banner and the strike-through card price, both driven by the shared endpoint.Screenshots:⚠️ NONE — the two rendered surfaces (
/m/[slug]banner + menu card strike-through) are pixel-identical by design: same numbers, same components, different data source. The only new rendering isCompareSavings, which no page references yet. Flagging it rather than shipping silently.Summary by CodeRabbit
New Features
Improvements