There is a function in almost every backend that maps an internal status code to a user-facing message. It starts as a tidy if/else block, and over the years someone has bolted on logging, metrics, and a redirect for one specific status. By the time you open it, it looks like this:

function resolveMessage(status, ctx) {
  if (status === 'trial_expired') {
    logger.info('trial_expired_shown', { userId: ctx.userId });
    metrics.increment('trial_expired_viewed');
    return { title: 'Your trial ended', cta: 'subscribe' };
  }
  if (status === 'payment_failed') {
    return { title: 'Payment failed', cta: 'retry' };
  }
  if (status === 'account_locked') {
    return { title: 'Account locked', cta: 'unlock' };
  }
  return { title: '', cta: null };
}

It works. It is not elegant. A reviewer will glance at it, approve, and move on.

Then the agent gets the task: “Refactor resolveMessage to be cleaner and easier to extend.”

What comes back is beautiful. A lookup object, a default, one function:

const MESSAGES = {
  trial_expired: { title: 'Your trial ended', cta: 'subscribe' },
  payment_failed: { title: 'Payment failed', cta: 'retry' },
  account_locked: { title: 'Account locked', cta: 'unlock' },
};

function resolveMessage(status, ctx) {
  return MESSAGES[status] ?? { title: '', cta: null };
}

TypeScript is green. The three existing unit tests pass. The PR description says “Replaced if/else chain with a lookup map for readability and extensibility.” It looks like a textbook cleanup.

Two weeks later, someone asks why the trial_expired_viewed metric has been flat.

The logging and the metrics call lived inside the trial_expired branch. The lookup map only holds display strings. When the agent extracted the data, it extracted the data and quietly dropped the side effect that was bolted onto one specific case.

This is the refactor trap that keeps showing up. The agent treats a branch as “data that belongs in a map.” It does not treat the branch as “a unit of behavior that may contain logging, telemetry, a redirect, a write, or a transaction.” The map version is shorter, more consistent, and strictly worse.

A few things make this hard to catch in review:

  • The diff looks like a pure simplification. Short diffs get approved fast.
  • The dropped side effect is on exactly one branch, so the other branches look identical before and after.
  • The unit tests only assert the returned object. They never asserted that the metric was incremented, because the logging was “obvious” in the old code.
  • TypeScript cannot tell you that a logger.info call disappeared.

The cheap guard I have started using: when an agent proposes collapsing branches into a map, a switch, or a strategy object, ask it to list every side effect that exists inside each branch and confirm whether each one was preserved, moved, or intentionally removed. If the answer is not explicit, the side effect was almost certainly dropped.

For telemetry specifically, the next step is to assert on the metric in the test. A test that only checks the rendered title will not catch this. A test that checks metrics.increment was called for trial_expired will.

The goal is not to forbid lookup maps. They are the right shape for this function. The goal is to make the side effect explicit instead of implicit. Either move the logging into a wrapper that runs for every status, or keep a small sideEffects map next to the messages map. What you cannot do is let the side effect live inside an if branch and then wonder why it vanished when the branch became data.