Expand PII redaction test coverage beyond the single happy-path case

[royalpinto007]

Why it matters

lib/utils/pii.test.ts currently has exactly one test: a single sentence containing one email and one US phone number. lib/utils/pii.ts handles considerably more than that, and its own doc comment says the patterns "may over-match in edge cases". Right now nothing in the suite pins down what that over-matching actually is, so a future tweak to PHONE_PATTERN could silently change behaviour on real submissions.

What to change

Extend lib/utils/pii.test.ts with cases that lock in current behaviour of redactEmails, redactPhones, and redactPii:

• Multiple emails in one string, all redacted.
• An international format such as +44 7700 900123.
• A digit run shorter than MIN_PHONE_DIGITS (7), which should be left alone, for example call 555-1212 versus ext 12.
• A long digit run that is not a phone number (an order ID, a token count). Write the assertion for what the code does today and add a short comment noting it is a known over-match, so the behaviour is documented rather than accidental.
• Text with no PII at all passes through unchanged.

Notes

• Run with npm test. Vitest is already configured via vitest.config.ts.
• If a case makes you think the regex itself is wrong rather than the test, say so in the PR description. That is a useful finding, not a problem.

Questions are very welcome. Comment here to claim it and ask anything you are unsure about, you will usually get a reply within a day.

[BabuBahir]

Copilot suggested the following input - output

"Multiple emails": "alice@example.com bob@domain.org" — expect both addresses redacted.
"International phone": "Call me at +44 7700 900123" — expect phone redacted.
"Short digit run vs phone": "call 555-1212 ext 12" — expect "555-1212" redacted, "12" unchanged.
"Long digit run (order ID/token)": "Order 12345678901234567890" — assert current code’s behaviour (document if it redacts).
"No PII": "This sentence has no PII." — expect unchanged.

will that be correct @royalpinto007 ?

[royalpinto007]

Those are good cases, especially multiple addresses in one string and international formats. Add them to lib/utils/pii.test.ts.

One ask: verify each expectation against the actual redaction output before committing, rather than trusting the suggested values. If any of them reveal a real gap in the redaction itself, that is a separate issue worth filing.

[BabuBahir]

hi @royalpinto007 as I am doing negative test case I would like to mention the below observation for 2nd test

// negative scenarios

  it("documents with less than 7 digit number notredacted", () => {
    // as  the Minimum digit count for something to qualify as a phone number is 7
    expect(redactPhones("Order 123456")).toBe(
      "Order 123456",
    );
  });


  it("doesnt redacts non-email addresses in one string", () => {
    expect(
      redactEmails("Contact _alice@example.com and bob@domain..org today"),
    ).toBe("Contact _alice@example.com and bob@domain..org today");
  })

The regex also passes _alice@example.com & bob@domain..org as emails

[BabuBahir]

But in any case in PII over-match can always a good thing :)

[royalpinto007]

@BabuBahir good catch, and you are right on both: _alice@example.com and bob@domain..org do get redacted today. That is the intended trade-off in pii.ts, over-redaction is safe and under-redaction leaks, so please don't assert they pass through untouched. Write them as documented over-match cases instead: assert the actual redacted output plus a one-line comment saying it is a known over-match. That is exactly the kind of case the issue is asking to pin down.

Your short-digit case is correct as written, Order 123456 is 6 digits so it stays.

Heads-up on coordination: @slegarraga has #23 open for this issue already. If you want to keep going, the two over-match cases above would make a clean follow-up PR on top of it.

[BabuBahir]

hi @royalpinto007 as told , I am testing cases of over-redaction and made these observations and need your insights

it("over-redacts a tracking number as a phone number", () => {
  expect(
    redactPhones("Tracking number: 9400111899223856928490"),
  ).toBe("Tracking number: [PHONE REDACTED]");
});


it("over-redacts an email with a numeric top-level domain", () => {
  expect(
    redactEmails("Contact bob@example.123 today")
  ).toBe(
    "Contact [EMAIL REDACTED] today",
  );
});

These two i expected to have the mentioned tobe assertions.
However the actuals respective values are

1. Tracking number: [PHONE REDACTED]90
2. Contact bob@example.123 today

[BabuBahir]

Anyways my final pii.test.ts is as follows 👍🏽

`import { describe, expect, it } from "vitest";
import { redactEmails, redactPhones, redactPii } from "./pii";

describe("redactPii", () => {
  it("redacts email addresses and phone numbers", () => {
    const input =
      "Reach me at person@example.com or +1 (415) 555-1212 before launch.";

    expect(redactPii(input)).toBe(
      "Reach me at [EMAIL REDACTED] or [PHONE REDACTED] before launch.",
    );
  });

  it("redacts every email in one string", () => {
    expect(
      redactEmails("Contact alice@example.com and bob@domain.org today"),
    ).toBe("Contact [EMAIL REDACTED] and [EMAIL REDACTED] today");
  });

  it("redacts international phone numbers", () => {
    expect(redactPhones("Call me at +44 7700 900123")).toBe(
      "Call me at [PHONE REDACTED]",
    );
  });

  it("leaves a short extension-only digit run alone", () => {
    expect(redactPhones("ext 12")).toBe("ext 12");
  });

  it("documents that an extension attached to a phone is redacted together", () => {
    // Known over-match: PHONE_PATTERN includes the `ext 12` suffix, so the
    // whole match is redacted rather than only the main number.
    expect(redactPhones("call 555-1212 ext 12")).toBe("call [PHONE REDACTED]");
  });

  it("documents that a long non-phone digit run is over-redacted", () => {
    // Known over-match: an order ID with enough digits is treated as a phone
    // number. Conservative redaction is intentional in the utility.
    expect(redactPhones("Order 12345678901234567890")).toBe(
      "Order [PHONE REDACTED]",
    );
  });

  it("passes text without PII through unchanged", () => {
    expect(redactPii("This sentence has no PII.")).toBe(
      "This sentence has no PII.",
    );
  });
});

it("documents with less than 7 digit number notredacted", () => {
  // as  the Minimum digit count for something to qualify as a phone number is 7
  expect(redactPhones("Order 123456")).toBe(
    "Order 123456",
  );
}); 

// over-redacts scenarios

it("over-redacts emails with invalid local-part separators", () => {
  expect(
    redactEmails(
      "Contact alice.@example.com and alice..smith@example.com today",
    ),
  ).toBe("Contact [EMAIL REDACTED] and [EMAIL REDACTED] today");
});

it("over-redacts emails with invalid domain separators", () => {
  expect(
    redactEmails(
      "Contact bob@-domain.org and bob@domain-.org today",
    ),
  ).toBe("Contact [EMAIL REDACTED] and [EMAIL REDACTED] today");
});

it("over-redacts emails with leading dots in the domain", () => {
  expect(redactEmails("Contact bob@.domain.org today")).toBe(
    "Contact [EMAIL REDACTED] today",
  );
});

it("over-redacts emails with consecutive dots in the domain", () => {
  expect(redactEmails("Contact bob@domain..org today")).toBe(
    "Contact [EMAIL REDACTED] today",
  );
});

it("over-redacts an email inside a URL", () => {
  expect(redactEmails("Visit https://alice@example.com/profile today")).toBe(
    "Visit https://[EMAIL REDACTED]/profile today",
  );
});

it("over-redacts a documentation email", () => {
  expect(redactEmails("Use user@example.com in the documentation")).toBe(
    "Use [EMAIL REDACTED] in the documentation",
  );
});

it("over-redacts multiple invalid email-like values in one string", () => {
  expect(
    redactEmails(
      "Values: _alice@example.com bob@domain..org carol.@example.com",
    ),
  ).toBe(
    "Values: [EMAIL REDACTED] [EMAIL REDACTED] [EMAIL REDACTED]",
  );
});

it("over-redacts a plain number as a phone number", () => {
  expect(redactPhones("Reference number: 1234567")).toBe(
    "Reference number: [PHONE REDACTED]",
  );
});

it("over-redacts a long numeric identifier as a phone number", () => {
  expect(redactPhones("Order ID: 001234567890")).toBe(
    "Order ID: [PHONE REDACTED]",
  );
});

it("over-redacts a version number as a phone number", () => {
  expect(redactPhones("Build version: 1.234.567")).toBe(
    "Build version: [PHONE REDACTED]",
  );
});

it("over-redacts an IP address as a phone number", () => {
  expect(redactPhones("Server IP: 192.168.1.1")).toBe(
    "Server IP: [PHONE REDACTED]",
  );
});

it("over-redacts a date-like value as a phone number", () => {
  expect(redactPhones("Reference date: 2026-08-06")).toBe(
    "Reference date: [PHONE REDACTED]",
  );
});

it("over-redacts a timestamp as a phone number", () => {
  expect(redactPhones("Created at: 20260806144500")).toBe(
    "Created at: [PHONE REDACTED]",
  );
});

it("over-redacts a postal code as a phone number", () => {
  expect(redactPhones("PIN code: 11000123")).toBe(
    "PIN code: [PHONE REDACTED]",
  );
});


it("over-redacts a currency amount as a phone number", () => {
  expect(redactPhones("Total amount: 123.456.789")).toBe(
    "Total amount: [PHONE REDACTED]",
  );
});

it("over-redacts coordinates as a phone number", () => {
  expect(redactPhones("Location: 28.6139 77.2090")).toBe(
    "Location: [PHONE REDACTED]",
  );
});

it("over-redacts an all-zero placeholder as a phone number", () => {
  expect(redactPhones("Placeholder: 000-000-0000")).toBe(
    "Placeholder: [PHONE REDACTED]",
  );
});

it("over-redacts a parenthesized value as a phone number", () => {
  expect(redactPhones("Value: (2026) 1234567")).toBe(
    "Value: [PHONE REDACTED]",
  );
});

it("over-redacts an extension-like value as a phone number", () => {
  expect(redactPhones("Extension: ext-1234568")).toBe(
    "Extension: ext-[PHONE REDACTED]",
  );
});

it("over-redacts a number embedded in an identifier", () => {
  expect(redactPhones("Serial number: ABC-1234567-XYZ")).toBe(
    "Serial number: ABC-[PHONE REDACTED]-XYZ",
  );
});

it("over-redacts multiple non-phone values in one string", () => {
  expect(
    redactPhones(
      "Date 2026-08-06, IP 192.168.1.1, version 1.234.567, order 001234567890",
    ),
  ).toBe(
    "Date [PHONE REDACTED], IP [PHONE REDACTED], version [PHONE REDACTED], order [PHONE REDACTED]",
  );
});

it("over-redacts an email embedded in a larger token", () => {
  expect(redactEmails("Contact prefixalice@example.comsuffix today")).toBe(
    "Contact [EMAIL REDACTED] today",
  );
});

// it("over-redacts a tracking number as a phone number", () => {
//   expect(
//     redactPhones("Tracking number: 9400111899223856928490"),
//   ).toBe("Tracking number: [PHONE REDACTED]");
// });


// it("over-redacts an email with a numeric top-level domain", () => {
//   expect(
//     redactEmails("Contact bob@example.123 today")
//   ).toBe(
//     "Contact [EMAIL REDACTED] today",
//   );
// });
 `

[royalpinto007]

Great digging @BabuBahir, and your two actuals are the interesting part.

Case 1 is a real bug, not just over-matching. Tracking number: [PHONE REDACTED]90 means the pattern consumed part of the 22-digit run and left 90 behind. Partial redaction is worse than none, because it looks handled while still leaking the tail. Please assert the ACTUAL current behaviour in your test and add a // known bug: comment above it, so the suite documents reality rather than hiding it. I have filed #58 for the underlying fix.

Case 2 is correct as-is. bob@example.123 is not a valid address (TLDs are not all-numeric), so leaving it alone is right. Assert the pass-through with a short comment saying why.

General rule for this file: assert what the code does today, and mark anything you believe is wrong with // known bug: plus an issue link. Never loosen an assertion to make it pass.

Your final test file looks good otherwise. Send the PR whenever you are ready.

[BabuBahir]

can you add these changes to your already raised PR @slegarraga , will save me some forking effort :D

[royalpinto007]

@BabuBahir go ahead and open your own PR for this, no need to wait.

That other PR is closed and its author is no longer active here, so nothing is coming from that direction. Your version is the more complete one anyway: it covers the multi-address and international cases, documents the two over-match behaviours, and is what turned up #47.

Fork, branch off main, and send it whenever you are ready. The issue is yours.

[BabuBahir]

I liked the name of this product @royalpinto007 agent-postmortem , I too have been also contributed to a catalogue, where AI slop Pull Request has hurt Open Source Maintainers . And this catalogue has this ai-contribution-policy.yml
Which lets project declare its AI-contribution policy once, in a form readable by humans, CI, forges, and AI coding agents. It is built on one design decision: verification, not detection.