Repository navigation
TASK-23054 copy polish round 2: no pronouns, sentence case, one amount format - #3446
Conversation
…t format
Integration QA found labels the my/your sweep missed ("Who can pay you",
"Let them pay by bank transfer", "You're withdrawing"). Hugo's rule is no
pronouns at all in titles, rows, buttons and labels, so the sweep now covers
you/them in all four catalogs, and the action-card header family changes
together so siblings still read alike.
- withdraw drops its "How would you like to withdraw?" heading: it repeated
the nav title, as the add-money one did
- profile menu and Edit profile move to sentence case
- the request payer screen and the receipt headline use formatBankAmount, so
round amounts lose ".00" (the receipt used to print "$12.5" too); a code
that stands in for a symbol gets a space ("ARS 13,500")
- the MXN unlock step reached from Add money keeps the Add money title
- the link card has one name: "Send with a link"
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
/chip review |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: peanutprotocol/peanut-ui/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
Code-analysis diffPainscore total: 8812.85 → 8856.09 (+43.24) 🆕 New findings (62)
…and 42 more. ✅ Resolved (63)
…and 43 more. 📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
The receipt PDF route is server code; importing formatBankAmount pulled in React hooks and broke the build.
There was a problem hiding this comment.
Chip review — changes requested
The new PDF formatter import breaks the production build; the merge-ref typecheck also fails after removal of mainHeading.
Findings
-
BLOCKING · src/app/receipt/[entryId]/pdf/receipt-pdf-model.ts:17 · Keep the PDF route out of the client hook module
The new import brings src/utils/currency.ts into the server PDF route, but that utility imports SYMBOLS_BY_CURRENCY_CODE from useCurrency.ts, which also imports React client hooks. The press-contract build fails with the Server Component useState/useEffect/useRef error; preview and screenshot builds fail downstream. Move the symbol table to a server-safe module (or otherwise make the formatter dependency server-safe) before using it in the PDF model. -
MAJOR · src/features/withdraw/views/WithdrawMethodView.tsx:50 · Update the remaining merged test call for the removed prop
Removing mainHeading from WithdrawMethodViewProps leaves a call in the supplied dev base test that still passes mainHeading. After the PR merges with that base, CI typecheck fails with TS2322 in WithdrawMethodView.test.tsx (the third render in the address-book test). Update that call as part of this PR or rebase and remove the stale prop there; the two render sites changed on this head are not enough. -
MAJOR · src/i18n/app/messages/en.json:810 · [claude-opus] Argentina rail copy drops the own-account-only rule
The rail description atsetup.residence.congrats.rails.arQrchanged from "QR payments and transfers with your own account" to "QR payments and bank transfers". The same edit was made in es-419 ("pagos QR y transferencias bancarias") and pt-BR ("pagamentos QR e transferências bancárias"). The product truth says this rail only works with the user's own account./home/chip/mono/product/quick-ref.md:106says "The Argentina rail cannot — own account only", andproduct/spending.md:21says "Argentine residents can withdraw to their own account only". There is also a known support problem:product/feedback/problems/third-party-transfers-rejected-confusing.mdrecords third-party ARS deposits that were silently rejected (2026-02-03 and 2026-07-15), and warns that the Manteca flow lacks a first-party warning. The new wording makes this worse because it reads as "bank transfers" in general. The same file still says "QR payments & own-account transfers" undercompare.items.arQr(line 833), so the app now describes the rail two different ways. The code is wrong here, not the product doc. Fix: restore the own-account wording without a pronoun, for example "QR payments and own-account bank transfers" / "pagos QR y transferencias desde cuenta propia" / "pagamentos QR e transferências de conta própria".
Checked clean
- Reviewed exact head against merge base and supplied dev base; no prior findings were supplied.
- Read exact-head CI failures: press-contract build and merge-ref typecheck; screenshot and preview failures cascade from build.
- Traced request bank amounts, receipt headlines, exact copy values, success-label callers, and changed locale placeholders.
- No additional reachable security or money-movement defect found in the changed display paths.
Security review: did not run — openrouter-http-402. This review is one reviewer short.
Third opinion by claude-opus: 1 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 7a4772c815ca · Context: repo, design · Took 8m
…n review) The Argentina rail keeps "own-account" (the rail refuses third-party transfers); the invite title stays a sentence; the cancel reason stays a question; "What it unlocks" replaces the generic "Features"/"Benefits"; "Requested from" reads with the name after it. Drops the last mainHeading prop the dev merge brought in.
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
No findings at this head. Prior findings P1, P2, and P3 are fixed.
Checked clean
- P1 fixed: the PDF formatter imports a server-safe currency-symbol table; exact-head press-contract build succeeded.
- P2 fixed: the remaining merged WithdrawMethodView test call no longer passes mainHeading; exact-head typecheck succeeded.
- P3 fixed: Argentina onboarding copy again limits transfers to own accounts across the affected locales, consistent with product documentation.
- Reviewed the merge-base diff, success-label callers, request and receipt amount formatting, locale copy, and security boundaries; no further reachable defect found.
- Unit, typecheck, and press-contract checks succeeded. The es-419 screenshot capture failed in the next/font Google font loader, outside changed code; other visual jobs were still running at review time.
Security review: did not run — openrouter-http-402. This review is one reviewer short.
Third opinion: did not run — it reads only the first review of a pull request; the first reviewer checks later rounds. This review is one reviewer short.
Exact head: b39d07987f43 · Context: repo, product, design, lexicon · Took 8m
🖼 Visual diff — 17 screens moved30 of 164 shots changed · 134 identical · baseline
job summary · before/after/diff images — artifact Fixture screenshots, no backend. Advisory — this check never blocks a merge. Posted from the default branch by ds-shots-comment.yml; the report it renders is untrusted data. |
|
English · Español · Español (Argentina) · Português (Brasil) After merge: 9a374bb → 4b47ee9. Capture complete in all locales. |
Copy and UI polish round 2 for TASK-23054, from integration QA (
local/scratch/qa-triage-2026-09-24/integration-qa.md). All four catalogs change together. Where a pronoun-free wording lost meaning, clarity won (release-captain review).What changes
formatBankAmount. No new formatter. The symbol map moved tosrc/constants/currency-symbols.consts.tsso the server PDF route can use the formatter without importing a React hook. The receipt used to print "$12.5"; it now prints "$12.50". A currency code used in place of a symbol gets a space: "ARS 13,500".tUnlockis removed.Checks
tsc --noEmit,prettier --check ., full Jest suite (one worker),screen-wordiness --check,npm run build.