Skip to content

adds pearls a receipt recording app - #144

Merged
zeeghazi merged 1 commit into
masterfrom
feature/pearl
Apr 22, 2026
Merged

adds pearls a receipt recording app#144
zeeghazi merged 1 commit into
masterfrom
feature/pearl

Conversation

@zeeghazi

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

Copy link
Copy Markdown

Code Review — PR #144: Pearl (receipt recording app)

Overview

Pearl adds a new mini-app that lets users compose a digital receipt (merchant, line items, taxes, payment method, notes, tags), preview it as a thermal-tape image, and permanently mint it on Arweave as a PNG + structured metadata. The architecture is clean — schema, constants, hooks, pure utilities, and UI components are all well-separated under core/features/pearl/. The Formik + Yup integration is solid, the live tape preview is a nice UX touch, and the re-use of Pinax's useUploadAndMint pipeline avoids duplicating upload logic.

That said, there are several issues worth addressing before merging.


Bugs / Correctness

1. Floating-point arithmetic for financial totals (high priority)

useComputedTotals.ts and LineItemRow.tsx both use raw JavaScript floats for money:

// useComputedTotals.ts
const bundle = i.children.reduce((s, c) => s + c.quantity * c.unitPrice, 0);
const taxes = receipt.taxLines.map(t => ({ amount: subtotal * t.ratePercent / 100 }));

// LineItemRow.tsx (inline, not going through the hook)
{(child.quantity * child.unitPrice).toFixed(2)}

JS floats produce the classic 0.1 + 0.2 = 0.30000000000000004 surprise. For displayed and on-chain financial values this matters — a user entering 3 × $3.33 will see $9.99 from toFixed(2) but the stored subtotal is 9.990000000000001. Consider working in integer cents throughout, or using a decimal library (e.g. decimal.js or dinero.js). At minimum, the child-row inline calculation on line 354 of LineItemRow.tsx should go through totals.itemTotal so all math lives in one place.

2. Sub-item removal restores unitPrice to 0, not null

// LineItemRow.tsx ~363
if (item.children.length === 1) {
  setFieldValue(`${namePrefix}.unitPrice`, 0);
}

The schema defaults unitPrice to null for parent items that have children (Yup.number().nullable().default(null)). When the last sub-item is removed the parent's unitPrice is set to 0 instead of null, so hasChildren becomes false and the unitPrice input is shown — but pre-filled with 0 rather than blank. This is likely not the intended UX.

3. Navigation guard wires to wrong state flags

// PearlPage.tsx ~1961
const { uploading, minting, transaction, minted } = useAppSelector(s => s.pinax);
useNavigationGuard({ uploading, minting, transaction, minted });

useMintReceipt wraps Pinax's hook but Pearl also introduces its own submitting local flag for the pre-upload rendering phase. A user who clicks "Save & Mint" and immediately tries to navigate away during the ~1s html-to-image render will not be guarded because pinax.uploading is still false at that point. submitting (or the derived busy) should be part of the navigation guard condition.

4. Missing file size validation

handleFilePicked sets the uploaded file without any size check. There is no limit before the file is handed to Pinax/Arweave. A large PDF could silently lead to unexpectedly high Arweave costs with no warning to the user.


Code Quality

5. plan.md committed as application code

src/alex_frontend/core/features/pearl/plan.md is a developer to-do list, not application code. It should live in a PR description, an issue, or a project board — not in the repo.

6. catch (e: any) in useMintReceipt.ts (~line 1233)

} catch (e: any) {
  const msg = e?.message ?? "Failed to render receipt image";

This suppresses TypeScript's unknown-catch safety. Prefer e instanceof Error ? e.message : "Failed to render receipt image".

7. Stale comment: "html2canvas" in renderTapeImage.ts

The function-level JSDoc says "html2canvas" but the implementation uses html-to-image. Minor, but confusing.

8. Unused CURRENCIES import in LineItemsEditor.tsx

CURRENCIES is imported and used only to look up the currency symbol for the column header. Since HeaderFields.tsx already renders the currency picker with the code, consider passing the symbol down as a prop or reading it from the same computed location — the import is fine but it is a duplication of the same symbol lookup that exists in Totals.tsx and TapePreview.tsx.


UX / Minor Issues

9. "Cancel" button label on the success screen is misleading

<Button variant="outline" scale="sm" onClick={onStartOver}>
  Cancel
</Button>

onStartOver resets the form and clears state — "Cancel" implies aborting an in-progress action. "Mint another" or "Start over" is more accurate.

10. Content filter doesn't distinguish Pearls from NFTs

The comment in ShelfDetail.tsx acknowledges this ("NFTs and Pearls are both stored as { Nft: tokenId } today, so selecting 'NFT' shows both"), but the filter label still says "NFT". Users may be confused when selecting "NFT" also shows their Pearl receipts. Even a small "(incl. Pearl)" note on the filter option would help until the canister gets a dedicated Pearl variant.

11. Missing Pearl-Payment-Note top-level tag

paymentNote is embedded in the Pearl-Data JSON blob but not surfaced as a top-level Arweave tag, unlike merchant, date, currency, etc. If future indexers want to filter/search by payment note this will require parsing the blob. This is a design choice, but worth flagging for intentionality.


Test Coverage

No tests were added. Two pure functions here are trivial to unit-test and would catch the float-arithmetic and tag-building edge cases:

  • computeTotals (edge cases: empty items, bundle items, zero-rate taxes)
  • buildArweaveTags (verify tag names, counts, JSON blob structure)

Summary

Area Status
Architecture / separation of concerns ✅ Clean
Formik + Yup integration ✅ Solid
Arweave tag design ✅ Well-structured
Financial arithmetic ⚠️ Float precision risk
Sub-item removal ⚠️ Restores wrong unitPrice value
Navigation guard during render phase ⚠️ Gap
File size validation ⚠️ Missing
plan.md in repo ❌ Remove
Test coverage ❌ None added

The foundation is strong. The float arithmetic issue is the most important fix before mint — a stored grand total of $9.990000000000001 written permanently to Arweave would be embarrassing. The rest are smaller polish items.

@zeeghazi
zeeghazi merged commit db1c3e6 into master Apr 22, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant