fix: apply request-money label research (TASK-22439) - #3137
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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: Path: .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: 9028.12 → 9028.12 (0) |
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
|
/chip review |
🖼 Visual diff — 17 screens moved21 of 84 shots changed · 63 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. |
![chip-peanut-bot[bot]](https://avatars.githubusercontent.com/in/2915219?s=60&v=4){kind=small}
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The Spanish request-action label is applied consistently to the app and generated marketing catalogs, inherited by es-AR, and protected without changing the English or Portuguese labels. No actionable defects found.
Checked clean
- Confirmed the detached worktree HEAD and merge base match the supplied head and base SHAs.
- Reviewed all three changed files and the app and marketing locale loaders; es-AR correctly inherits the es-419 navigation label.
- Regenerated every marketing catalog in memory and confirmed the committed subsets exactly match their source catalogs; changed JSON parses and the diff passes whitespace checks.
- Checked the live Lexicon and linked Notion task; the Lexicon has no canonical request-action label, while the task scope requires distinguishing Request from Receive and Add money.
- Verified the cited official Revolut Spain and PicPay help pages use Solicitar dinero and Cobrar respectively for creating payment requests.
- Exact-head aggregate CI is green; the advisory ds-shots job was still in progress at review time.
Security review: did not run — this change has no security, privacy or money surface, so it was not asked. This review is one reviewer short.
Third opinion: did not run — claude-api_error. This review is one reviewer short.
Exact head: 24870f7fe5ce · Context: repo, product · Took 8m
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The Spanish request-action label is applied consistently to the app and generated marketing catalogs, inherited by es-AR, and protected without changing the English or Portuguese labels. No actionable defects found.
Checked clean
- Confirmed the detached worktree HEAD and merge base match the supplied head and base SHAs.
- Reviewed all three changed files and the app and marketing locale loaders; es-AR correctly inherits the es-419 navigation label.
- Regenerated every marketing catalog in memory and confirmed the committed subsets exactly match their source catalogs; changed JSON parses and the diff passes whitespace checks.
- Checked the live Lexicon; it has no canonical request-action label that conflicts with this scoped editorial change.
- Verified the cited official Revolut Spain and PicPay help pages use Solicitar dinero and Cobrar respectively for creating payment requests.
- Exact-head required aggregate CI is green. The advisory ds-shots job failed only because the unchanged full design-system component sweep exceeded its 90-second timeout after all localized landing-entry cases passed; the changed copy does not reach that failing path.
Security review: did not run — this change has no security, privacy or money surface, so it was not asked. This review is one reviewer short.
Third opinion: did not run — claude-api_error. This review is one reviewer short.
Exact head: 24870f7fe5ce · Context: repo, product · Took 4m
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
No actionable defects found. The Spanish request label, locale inheritance, generated marketing catalog, and per-route showcase test split are coherent; required exact-head CI is green.
Checked clean
- Confirmed the detached worktree HEAD, merge base, trusted author, dev base ref, and supplied base/head SHAs.
- Checked app and generated marketing catalogs: es-419 resolves request to Solicitar, es-AR inherits it, and English/Portuguese plus add-money labels remain distinct.
- Checked the linked product task and live Lexicon. The task calls for locale-specific research; the Lexicon has no conflicting definition for this navigation label.
- Checked generator parity without modifying the worktree; every committed marketing catalog matches scripts/generate-marketing-messages.js output.
- Checked the showcase split for unique routes, per-route capture, console assertions, retries, parallel execution, and the workflow timeout envelope.
- Required exact-head aggregate CI and unit, typecheck, format, ESLint, native export, and screen tests passed. Advisory ds-shots was still capturing screenshots when reviewed.
- The separate screen-library capture failed identically on base and head because its shared harness lacked scripts/screens/run-capture.mjs, so it is not caused by this diff.
- Focused local Jest and Playwright listing could not run because the detached review worktree has no installed test binaries; read-only Node catalog and generator checks passed.
Security review: did not run — this change has no security, privacy or money surface, so it was not asked. This review is one reviewer short.
Third opinion: did not run — claude-api_error. This review is one reviewer short.
Exact head: d5e0e7a1e412 · Context: repo, product · Took 8m
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
No actionable defects found in the Spanish request-label update or the per-route showcase test split.
Checked clean
- Confirmed the detached worktree, trusted author, dev base, base SHA, and exact head SHA match the supplied review scope.
- Verified navigation.request changes only from Recibir to Solicitar in es-419, es-AR inherits that value, and English, Portuguese, and add-money labels remain distinct.
- Compared every generated marketing catalog in memory with its committed file; all catalogs match their source subsets byte-for-byte.
- Verified the showcase rewrite deduplicates routes and preserves the response, modal, capture, and console-error assertions while giving each route the configured 90-second timeout and retry.
- Checked the live Lexicon and TASK-22439 scope; the Lexicon does not define this label, and the task requires evidence-based locale recommendations without inventing a separate Argentine term.
- Exact-head code analysis and ownership checks are green; the Tests, Screen library, and preview workflows were still pending. Focused local Jest and Playwright execution was unavailable because the detached worktree has no installed dependencies.
Security review: did not run — this change has no security, privacy or money surface, so it was not asked. This review is one reviewer short.
Third opinion: did not run — claude-api_error. This review is one reviewer short.
Exact head: 1284aeff2e16 · Context: repo, product · Took 8m
1 participant
Summary
Spanish navigation currently calls a payment request “Recibir”, which describes receiving funds rather than creating a request. Use Solicitar in es-419 and inherited es-AR. Preserve Cobrar in pt-BR and Request in English.
Replace #3066 with a fresh branch based on
dev. Regenerate the marketing catalog so its navigation uses the same label. Keep funding labels Agregar / Adicionar separate. Test the resolved navigation labels across all four locales, including the es-AR overlay.Task and research
TASK-22439 compared eight official app sources. This is an editorial recommendation, not a measured comprehension result or market-share survey.
Risk and scope
Low-risk navigation copy change. No payment logic, API contract, or data handling changes. Longer Spanish text needs visual verification.
Validation
1284aeff2: required CI, unit tests, TypeScript, formatting, ESLint, native export, and preview deployment passed.claude-api_error.Remaining shared screen-library failures
The separate Screen library run now finds its harness after syncing dev. Both base and head captured 184 screens but timed out on
16-a-iospwainstallmodal. The publisher also reportsCloudflare preview original mismatch or unavailable. These remain unresolved; they are separate from the passing ds-shots job. Do not treat the screen-library pipeline as verified.Separate content follow-up
The documentation review found two mismatched button instructions:
content/help/request-money/es-419.mdsays “Cobrar”; it should say “Solicitar”. The Portuguese articlecontent/help/request-money/pt-br.mdsays “Solicitar”; it should say “Cobrar”. The Portuguese mismatch predates this PR. These belong in a separate content change.No architectural smells added. Product behavior is unchanged. The privacy-policy review found no legal impact.
Showcase timeout fix
The advisory ds-shots job timed out because one test visited every design-system page within a shared 90-second budget. Each unique route now has its own test, timeout, retry, console assertion, and capture. Duplicate navigation links are tested once.
Validation: the preview-mode local production build passed; all 53 showcase tests passed in 1.0 minute at 375×667 with two workers. Prettier passed. ESLint excludes this existing E2E path. CI and Chip verified the synchronized head; the shared screen-library failures are listed above.