A correct validation rule almost broke a request our existing customers had been making for months

tags: webdev, typescript, postgresql, api

You've shipped this exact change before, probably more than once: a feature that's the obvious mirror image of one you already have. You already flag features as "dead" after N days of no events - customers ask for the opposite, a "new" flag for features that just started getting used. Same table, same kind of threshold field, same UI slot. It reads like an afternoon of work, because the hard part - detecting a feature's usage lifecycle from raw events - is already solved. You're just adding the other half of a pair.

That's exactly the case that's dangerous, because "the other half of a pair" is where code quietly assumes the pair will always relate to each other the way you're imagining right now, in the twenty minutes you're spending on this. We added the mirror image of an existing feature, wrote what looked like an obviously correct validation rule for it, and that rule would have rejected a request our existing customers had already been making successfully for months - a request that had nothing to do with the new field at all.

The feature: nothing surprising

Eventra flags a feature "dead" when it's had no events for N days - 14 by default, configurable per project. We added "new": a feature is "new" for N days after its first event - 7 by default, same per-project override. Same table, same kind of field, same kind of badge next to it in the UI. The actual detection logic is maybe fifteen lines: swap lastUsed for firstUsed, flip a comparison. This part shipped exactly as boring as it sounds.

The two thresholds have to stay in a specific relationship, though: a feature can't reasonably be "new" (fired for the first time recently) and "dead" (hasn't fired in a while) at the same moment, which becomes possible the instant someone configures the new-threshold larger than the dead-threshold. So the project-settings endpoint got a check:

if (nextNewThreshold > nextDeadThreshold) {
  throw new BadRequestException(
    'newFeatureThresholdDays must be less than or equal to deadFeatureThresholdDays',
  );
}

Read it on its own and there's nothing wrong with it. It's the correct invariant, stated directly, rejected with a clear message. It's the kind of line that sails through review because reviewing it means checking "is this logically true," and it is.

The part that isn't visible from the rule itself

Here's what the rule doesn't show you: the dead-threshold field already existed, already had months of live customer configurations, and its allowed minimum is 1 day. The new-threshold field is brand new, and every existing project got it from a migration with a default of 7 days - a default nobody chose, stamped onto every account retroactively, including accounts that will never open the settings page and notice it exists.

Now replay an entirely ordinary support scenario. A customer who set their dead-feature threshold to 5 days three months ago - because their product ships fast and features go stale quickly - opens settings today and tries to lower it to 3 days. Same field they've edited before, same kind of change they've made before. The request now fails with a 400 about newFeatureThresholdDays - a field this customer has never seen, sitting at a default value they never chose, silently in conflict with the one field they're actually trying to change.

That's not a hypothetical edge case you'd need bad luck to hit. It's the default outcome of "add a symmetric field with a fixed default, then validate it against a pre-existing field whose live values already span a wider range than the new field's default." It passes every unit test that only exercises the new field's own valid range. It passes code review, because the validation logic is genuinely correct. It only surfaces the first time a real customer, with a low-enough pre-existing value in the other field, touches that other field - which for a field that's been configurable for months, is not a matter of if.

The fix: tell the two cases apart

The rule was conflating "the user asked for an invalid new-threshold" with "the new-threshold just happens to be sitting in the way of something else they asked for." Those need different responses:

if (nextNewThreshold > nextDeadThreshold) {
  if (dto.newFeatureThresholdDays !== undefined) {
    // they explicitly asked for a value that doesn't fit - say so
    throw new BadRequestException(
      'newFeatureThresholdDays must be less than or equal to deadFeatureThresholdDays',
    );
  }
  // they only touched deadFeatureThresholdDays - the new field was never
  // part of their request, so adjust it instead of rejecting theirs
  nextNewThreshold = nextDeadThreshold;
}

If someone explicitly sets an invalid newFeatureThresholdDays, they still get told exactly why, immediately. If they never mentioned it and it just happens to sit above whatever they're lowering deadFeatureThresholdDays to, it gets quietly pulled down to match instead - the request that always worked keeps working, and the invariant still holds afterward either way.

That covers every request from here on. It doesn't cover the rows already sitting in the database with the same conflict, from before the check existed - a customer at dead=5 already has new=7 sitting there right now, invalid, untouched, waiting for the day they edit either field and get a result they didn't ask for. So the migration that added the field got a follow-up: a plain backfill, applied once, capping the new field wherever it already exceeded the old one.

UPDATE "Project"
SET "newFeatureThresholdDays" = "deadFeatureThresholdDays"
WHERE "newFeatureThresholdDays" > "deadFeatureThresholdDays";

Without it, the code-level fix only protects the next edit. The invariant would still be false, quietly, for every account that happens to already be in that state - correct from here forward is not the same claim as correct right now, and a migration is the only place you get to make the second claim true retroactively.

What actually catches this

Nothing about this bug shows up by testing the new field in isolation. A fresh test suite's fixtures are exactly as consistent as whoever wrote them - the fixtures for a brand-new field are never going to include "an old field, from a real account, configured to a value from before this field existed," because you have to go looking for that scenario on purpose. The question that surfaces it isn't "does my new validation work for valid inputs to the new field," it's "for every row that already exists, is my new invariant already true, or could it already be violated before anyone touches anything" - and that second question only gets asked if you go look at what's actually sitting in the table, not just at what the migration's default value claims.

The concrete version of that, for a schema change specifically: after writing the migration, don't just check that it applies. Check what the data looks like the moment after it applies, for every row that was there before it ran, against whatever new rule you're about to enforce on top of it. If any existing row could already fail that rule, the rule needs a plan for those rows before it ships, not just for the next request that happens to touch them.

Try it

The "new" feature status is live on eventra.dev now - features get flagged "new" for their first 7 days, configurable per project, right next to the existing dead-feature detection. If you want to see it on real data: the homepage has a "Try live demo" link, no signup, straight into a populated workspace.