Skip to content

More unit testsfor pii - #48

Merged
royalpinto007 merged 1 commit into
AgentPostmortem:mainfrom
BabuBahir:MoreUnitTestsforPii
Aug 6, 2026
Merged

More unit testsfor pii#48
royalpinto007 merged 1 commit into
AgentPostmortem:mainfrom
BabuBahir:MoreUnitTestsforPii

Conversation

@BabuBahir

@BabuBahir BabuBahir commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

What & why

Add more tests too test over-redacting PII redactEmails and redactPhones . As PII information is sensitive so better to over redact than under redact.

Changes

Add tests. Improvised Phone Number regex

Checklist

  • Focused change with a clear description
  • Ran format / lint / tests (see README)
  • Updated docs or CHANGELOG if relevant
  • Linked any related issue (e.g. Closes #123)

Resolves #/issues/19
Fixes #/issues/47

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

Welcome, @BabuBahir, and thanks for your first pull request to agent-postmortem.

A quick look at CONTRIBUTING.md covers how things are set up and run here.

I'll review this shortly. Ask anything in the thread if something is unclear.

@BabuBahir
BabuBahir force-pushed the MoreUnitTestsforPii branch from d812838 to 6bdd7f3 Compare August 6, 2026 10:28
@BabuBahir
BabuBahir marked this pull request as ready for review August 6, 2026 10:29
@royalpinto007

Copy link
Copy Markdown
Contributor

This is a genuinely good contribution @BabuBahir, thank you.

You found the bug yourself while writing tests, traced it to the right cause, and then shipped the fix with both boundary assertions rather than just the one. The (?<!\d) matters as much as the trailing (?!\d), since without it the engine just shifts the start rightward and orphans digits at the front instead of the back.

The // Known over-match: comments are the part I like most. Documenting real behaviour beats quietly loosening an assertion, and now the next person reading this file knows those cases are deliberate rather than accidental.

All checks green. Merging, and this closes #47 as well.

A star on the repo is appreciated if you enjoyed it, and there are plenty more open issues, including some harder ones now if you want a bigger bite.

@royalpinto007
royalpinto007 merged commit dea006e into AgentPostmortem:main Aug 6, 2026
3 checks passed
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

Merged, @BabuBahir. Thank you for taking the time.

Contributions like this are what keep agent-postmortem moving.

A star on the repo would mean a lot if this project is useful to you.

Happy to see more from you whenever you have the time.

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.

2 participants