Review Agent PRs That Bill Orders With Floats

A Friday checkout pull request looked finished at first glance. The agent had wrapped the charge call in retries. The diff was small and the tests were green.

The reviewer opened the money helper before approving. Prices arrived as JavaScript binary floating point numbers. A later retry could post the same order twice.

That pair of choices is a common agent miss. The patch optimizes only the visible happy path. It skips ledger rules that production billing needs.

What the pull request changed

The branch added a charge helper to the Node service. It read the amount field from the request body. It converted that field with Number and called the provider.

async function chargeOrder(req) {
  const amount = Number(req.body.amount);
  const result = await provider.charge({
    orderId: req.body.orderId,
    amount,
  });
  return { ok: true, result };
}

A second commit retried any thrown error three times. The retry path had no idempotency key at all. There was no check that amount stayed integer cents.

async function chargeWithRetry(payload) {
  for (let attempt = 1; attempt <= 3; attempt += 1) {
    try {
      return await provider.charge(payload);
    } catch (error) {
      if (attempt === 3) throw error;
    }
  }
}

Green unit tests only mocked a successful charge. Those unit tests never replayed a provider timeout. They never asserted the stored integer minor units.

What to trust in the diff

Trust the narrow intent when the route contract is clear. A checkout handler should accept an order id. It should charge once and return a stable status.

Trust new tests that fail before the fix. Trust types that keep money in integer minor units. Trust a provider client that forwards an idempotency key.

Trust comments only when a test locks the same rule. An agent often explains a guard it did not add.

What to revert before merge

Revert float conversion of prices, tax, and totals. Binary floating point cannot represent every decimal cent. Adding one tenth and two tenths is not exact.

Revert blind retries that wrap payment provider posts. A timeout does not prove the provider skipped the charge. The next attempt can bill the customer again.

Revert catch blocks that ignore the error object. The sample swallows the first two provider failures. Operators then lose the provider code and request id.

Revert request-body amounts as the billing source of truth. The client can send a different total than the cart. The server should recompute minor units from stored line items.

Numbered review steps

1. Open the diff and list every money field. Mark each field as minor units, decimal string, or binary float. Treat every binary float field as a revert candidate.
2. Trace one order id from the route to the provider call. Confirm the same key is stored before the network call. Confirm a replay returns the first stored result.
3. Read the retry loop and name each caught error. Timeouts, conflicts, and server errors need different paths. A single loop for all errors is not enough.
4. Check the test files for risky boundary amounts. Include a zero total, a one-cent line, and a distorted float total. Include a timeout followed by a duplicate post.
5. Run the suite on a clean machine, not only in the agent log. A free server is enough for this small Node service. Record the command and the exit code in the review.
6. Write review comments that name the failing case. Ask for a patch, not a prose promise. Merge only after new tests fail on old code and then pass.

Commands that make the review reproducible

Check out the branch and limit the diff to billing files. The commands below are an example workflow, not a live pull request. Replace the branch name with the branch under review.

git diff --unified=3 main...HEAD -- src/charge.js test/charge-once.test.js
node --test test/charge-once.test.js

Save the exit code next to the review note. A green run on old tests does not approve the design. The new cents test must exist and must fail on the old helper.

A decision table for the reviewer

Diff signal	Trust for now	Revert or block	Test to add
Integer cents from stored lines	Yes, if a test locks the formula	No	One-cent and large quantity totals
Number(req.body.amount) sent onward	No	Yes	Reject non-integer input
Retry without an idempotency key	No	Yes	Timeout, then one duplicate post
Key stored before the provider call	Yes, if unique per order charge	No	Replay returns the prior status
Empty catch around provider errors	No	Yes	Assert the logged provider code
Mock that only returns success	No	Expand the tests	Failure and replay cases

Use this table as the reusable review artifact. Copy each row into the pull request comment. Check every row against the lines in the diff.

A minimal regression to demand

The replacement helper below is a review target, not a production billing engine. The helper stores order money as integer cents. It refuses a second charge when the order already has a provider reference.

function isDigits(text) {
  return text.length > 0 && [...text].every((ch) => ch >= '0' && ch <= '9');
}

function toCents(decimalText) {
  const parts = String(decimalText).split('.');
  const major = parts[0];
  const minor = parts[1];
  const valid = parts.length === 2 && isDigits(major) && minor
    && minor.length === 2 && isDigits(minor);
  if (!valid) throw new Error('amount must be dotted cents');
  return Number(major) * 100 + Number(minor);
}

async function chargeOnce(order, provider, store) {
  const existing = await store.getCharge(order.id);
  if (existing && existing.providerRef) return existing;

  const amountCents = order.lines.reduce((sum, line) => {
    return sum + toCents(line.unitPrice) * line.qty;
  }, 0);

  const row = existing || await store.saveCharge({
    orderId: order.id,
    amountCents,
    idempotencyKey: `order:${order.id}:charge`,
    providerRef: null,
  });

  const result = await provider.charge({
    amountCents: row.amountCents,
    idempotencyKey: row.idempotencyKey,
  });

  return store.attachRef(order.id, result.providerRef);
}

module.exports = { chargeOnce, toCents };

Label the snippet above as a proposed patch. It was not executed against a live payment provider here. Run it only after adapting it to a fake provider in CI.

A compact test plan fits in one proposed file. Import the helpers from the module under test first. The file should fail on the float helper and pass on the replay patch.

const test = require('node:test');
const assert = require('node:assert/strict');
const { chargeOnce } = require('./charge-once');

test('recomputes cents and does not charge twice', async () => {
  const calls = [];
  const provider = {
    async charge(payload) {
      calls.push(payload);
      return { providerRef: 'prv_1' };
    },
  };
  const memory = new Map();
  const store = {
    async getCharge(id) {
      return memory.get(id) || null;
    },
    async saveCharge(row) {
      memory.set(row.orderId, row);
      return row;
    },
    async attachRef(id, providerRef) {
      const row = memory.get(id);
      row.providerRef = providerRef;
      return row;
    },
  };
  const order = {
    id: 'ord_9',
    lines: [{ unitPrice: '0.10', qty: 2 }],
  };

  const first = await chargeOnce(order, provider, store);
  const second = await chargeOnce(order, provider, store);

  assert.equal(first.amountCents, 20);
  assert.equal(second.providerRef, 'prv_1');
  assert.equal(calls.length, 1);
});

Treat that test file as an unexecuted example. The assertion on call count is the replay lesson. A timeout injection test should still be added later.

A strict cents check also rejects distorted float totals. Repeated one-tenth prices are a typical failure case. Keep that case in the suite beside the replay test.

Failure cases the old tests hide

A timeout after the provider accepts the charge is the costly case. The client sees an error and the agent loop tries again. Without a stored key, both posts can capture funds.

A decimal string such as ten point one zero becomes a binary number. Multiplied by a quantity, the total can drift by a fraction. Providers that expect integer minor units may reject or mis-round.

A cart edit between page load and submit changes the true total. The request body still carries the stale browser amount. Recomputing from stored line items closes that gap.

A comment that asks for a real patch

Paste a comment that names the file, the rule, and the test. Vague praise does not block a bad merge. The example below is text for the reviewer, not model output.

src/charge.js: amount uses Number, and chargeWithRetry has no idempotency key.
Please store integer cents from line items, save a key before the provider call, and add a node:test that asserts one provider call after a replay.
I will re-review when that test fails on this commit and passes on the fix.

Where a free coding setup fits

MonkeyCode free model access can draft the first review pass. It can draft comments from the billing diff. Disclosure: This article was prepared as part of MonkeyCode's product outreach.

The final review decision stays with a human. A free server from that offering can run the test. Use a clean checkout rather than the agent workspace.

Confirm current free-tier limits on the product page before a review sprint. This article does not claim a token quota. It also omits hardware size and offer duration.

Use the model output as a checklist, not as an approval. Paste that decision table into the model prompt. Ask the model to map each row to file names and line ranges.

Then open those lines and confirm the mapping by hand. Reviewers can try this checklist on the next billing diff. That is a workflow trial, not a launch announcement.

Limitations

Integer cents do not solve currency rules by themselves. Some currencies use no minor-unit digits at all. Others use three digits after the major unit.

The dotted-cents parser above fits only two-digit minor units. Very large major units can exceed the safe integer range. A production helper should use a decimal library for those cases.

Idempotency keys must match the current provider contract. A guessed header name will not dedupe a charge. Read the provider's current API reference before copying the snippet.

The in-memory store will not survive two app instances. Production needs a unique constraint on the order id. This article does not provide that database migration.

The sample never handles partial refunds or tax by jurisdiction. Captured and authorized payment states are also out of scope. Teams with those flows need a ledger design, not this helper.

Free hosted runs can differ from a locked CI image. A passing free-server log is not a release certificate. Re-run the same command in the repository's required checks.

Who should skip this approach

Skip this pattern for card data, bank debits, or regulated payment flows. Those systems need a separately reviewed payment integration. They do not need an agent patch plus a blog snippet.

Skip it when the team cannot recompute totals from stored lines. Client-supplied amounts will keep drifting from the cart. Fix the cart model before reviewing the charge helper.

Skip free-tier execution for secrets, live keys, or customer payloads. Use a fake provider and synthetic orders only. A review machine is not a production vault.

Skip model-written approvals when no human reads the payment diff. The tool can list risks in the changed files. It cannot own the final merge on its own.

Closing the review

The Friday patch was incomplete rather than malicious. Float math and blind retries can double-charge a customer. That risk is enough to block the merge.

Trust the order contract and any test that fails on the old path. Revert binary money and retries that lack an idempotency key. Demand the cents test and the single-charge replay before merge.