Skip to content

fix(tinyredact): keep a partial overlap's tail redacted - #310

Merged
M3gA-Mind merged 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:fix/pii-merge-partial-overlaps
Oct 23, 2026
Merged

M3gA-Mind merged 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:fix/pii-merge-partial-overlaps

Conversation

@M3gA-Mind

@M3gA-Mind M3gA-Mind commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

When two detectors matched overlapping ranges, dedupe_overlaps kept the earlier one and dropped the other entirely. For a partial overlap, for example hits at bytes 0..10 and 5..15, bytes 10..15 were left in clear text.

The fix merges an overlapping hit into the kept one. The earlier, longer hit keeps its token, and the combined range is redacted once. That fails closed, and nothing is redacted twice.

Related issue

None. Found in an edge-case audit of the redaction engine.

API or behavior changes

No API change. A partial overlap is now redacted up to the end of the later hit, under the first hit's token. A fully contained overlap behaves as before.

Validation

Commands actually run, with their outcome:

  • N/A: cargo fmt --all -- --check. Ran rustfmt (edition 2024) on the two touched files only. CI runs the full check.
  • N/A: cargo clippy --all-targets --all-features -- -D warnings. Left to CI; only the regression test runs locally.
  • N/A: cargo build --all-targets --all-features. Left to CI.
  • cargo test -p tinyredact --lib a_partial_overlap_keeps_its_tail_redacted:
    • with main's dedupe_overlaps: FAILED, 0 passed / 1 failed, left: [(0, 10, CREDIT_CARD)], right: [(0, 15, CREDIT_CARD)].
    • with the fix: 1 passed.

Tests

New a_partial_overlap_keeps_its_tail_redacted: hits (0..10) and (5..15) → one hit (0..15) carrying the first token.

Documentation

The comment on dedupe_overlaps now describes the merge.

Checklist

  • The change is focused on one logical change
  • No new #[allow(...)], #[ignore], or relaxed lints
  • No secrets, tokens, or .env contents in the diff or the description

Summary by CodeRabbit

  • Bug Fixes
    • Overlapping sensitive-data matches are now redacted across their full combined range, rather than leaving part of the overlap uncovered.

dedupe_overlaps dropped any hit that overlapped an earlier kept hit, so
when two detectors matched overlapping ranges (0..10 and 5..15) the
bytes 10..15 were left in clear text. Merge the overlapping hit into the
kept one instead: the earlier, longer hit keeps its token and the
combined range is redacted once.
@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3fe69f08-6de8-4bc9-b906-b3ed360da051
📥 Commits

Reviewing files that changed from the base of the PR and between 4f5145c and 528e35c.

📒 Files selected for processing: 2
  • crates/tinyredact/src/pii/mod.rs
  • crates/tinyredact/src/pii/mod_tests.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

dedupe_overlaps now merges overlapping hit ranges into the earlier hit, retaining its token and extending its end. A regression test checks that overlapping credit-card and phone hits produce one combined range.

Changes: PII overlap deduplication

Layer / File(s) Summary
Merge overlapping hit ranges
crates/tinyredact/src/pii/mod.rs, crates/tinyredact/src/pii/mod_tests.rs
dedupe_overlaps extends the earlier hit to cover the end of an overlapping hit. Non-overlapping hits are still appended. A regression test checks that overlapping credit-card and phone hits produce one credit-card hit spanning (0, 15).

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: senamakel

Merge Risk: ⚪ Minimal · up to 528e3

The partial-overlap tail is covered, with no identified issue requiring a fix before merge.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: preserving the tail of a partially overlapping redaction range.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit checks each span with care,
Two crossing hits now join as one.
The first token stays in place,
Its range grows to cover both.
A test confirms the merged span,
Then hops away beneath the moon.

Comment @coderabbitai help to get the list of available commands.

@tinysweeper

tinysweeper Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Reviewed head: 528e35c14e5d31a8f49a0d8eac83997d23395eb1

Verdict: Ready for maintainer review · Priority: none
Coverage: Reviewed 2 of 2 files.
Pull request purpose (title): fix(tinyredact): kee…

Diagnostics and supporting details

Full summary

Pull request purpose (title): fix(tinyredact): keep a partial overlap's tail redacted. Tiny Sweeper completed its review; deterministic results follow.

Review snapshot

Change surface Files Review signal Count
Production 1 Active findings 0
Tests 1 Noted findings 0
Documentation 0 Resolved findings 0
Configuration 0 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

`dedupe_overlaps` in `crates/tinyredact/src/pii/mod.rs` no longer discards a hit whose range overlaps the previously kept hit. Instead, when the incoming hit starts before the kept hit's end, the kept hit's end is extended to the maximum of the two ends (`k.end = k.end.max(h.end)`); otherwise the hit is pushed as before. The ordering (start ascending, length descending) is unchanged, so the earlier, longer hit retains its token while the combined range is redacted once and no double redaction occurs. A doc comment now describes the merge, and a new unit test `a_partial_overlap_keeps_its_tail_redacted` in `crates/tinyredact/src/pii/mod_tests.rs` constructs hits at 5..15 (PII_PHONE) and 0..10 (PII_CC), runs `dedupe_overlaps`, and asserts the result collapses to a single (0, 15, PII_CC) range. Net change is 26 additions and 5 deletions across two files. No API changes are introduced.

Features

  • Modified — dedupe_overlaps merges partially overlapping hits instead of dropping the later one: Previously a partially overlapping later hit was dropped, so bytes between the kept hit's end and the overlapping hit's end were left unredacted. The kept hit's end is now extended to cover the full partial overlap, failing closed: the combined range is redacted once under the earlier, longer hit's token, and the monotone max cannot shrink a range or reintroduce a dropped hit. (crates/tinyredact/src/pii/mod.rs fn dedupe_overlaps(hits: &mut Vec<Hit>) {, crates/tinyredact/src/pii/mod.rs fn cc_keyword_near(norm: &str, start: usize, end: usize) -> bool {)
  • Added — Unit test pinning merged-range behavior for a partial overlap: A new test constructs overlapping hits (0..10 PII_CC and 5..15 PII_PHONE) and asserts the merged result is a single (0, 15, PII_CC) range, directly pinning the merge behavior and the first token's retention on the function under change. (crates/tinyredact/src/pii/mod_tests.rs fn a_separated_digit_list_with_no_card_is_left_alone() {)
  • Modified — Updated doc comment on dedupe_overlaps: The comment now states that overlapping hits are merged into the kept hit, that the earlier and longer hit keeps its token, that a partial overlap's tail is still covered (fails closed), and that nothing is redacted twice — aligning the documented contract with the implementation. (crates/tinyredact/src/pii/mod.rs fn dedupe_overlaps(hits: &mut Vec<Hit>) {)

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

No active actionable findings.

Before merge

None.

Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The critique lane answered and found no defect introduced by this diff, characterizing the merge as a real behavior change that closes an under-redaction gap (splice_redactions skips a hit whose start_orig is below the cursor, so an old partially overlapping earlier+shorter hit could leave its tail unredacted) and noting the merge is monotone and cannot shrink a range or drop a hit.
  • Lane summary: The lookup confirms the merged-range change is a real behavior change rather than a no-op: `splice_redactions` maps each hit's normalized offsets back to the original and skips a hit whose `start_orig < cursor`, so with the old drop-on-overlap rule a partially-overlapping earlier+shorter hit could leave its tail bytes unredacted, and the new merge closes that gap. The merge is monotone (`k.end = k.end.max(h.end)`) and the sort guarantees every later hit starts at or after the kept hit's start, so it cannot shrink a range or drop a hit; the one test added (`a_partial_overlap_keeps_its_tail_redacted`) asserts the intended earlier+longer-token/merged-tail outcome. I found no defect introduced by this diff. _The code index is behind this pull request (indexed at `1f4681b009b7`), so retrieved context may be out of date._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The security lane answered and reports the change fails closed: the kept hit only ever grows, merged ranges stay covered, the sort invariant is preserved, and no untrusted-input, auth, network, or CI surfaces are touched.
  • Lane summary: The change to `dedupe_overlaps` replaces dropping an overlapping hit with extending the kept hit's end, which fails closed and cannot split a span: the kept hit only ever grows, so merged ranges stay covered. Sorting is unchanged (start ascending, length descending), so the invariant that a later hit cannot start strictly inside an earlier kept hit while extending beyond it holds. The new test pins the merged-range behaviour. No untrusted-input, auth, network, or CI surfaces are touched. _The code index is behind this pull request (indexed at `1f4681b009b7`), so retrieved context may be out of date._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The tests lane answered and reports the added test is real and pins the new merging behavior, and flags the widened PII_CC range over formerly PII_PHONE bytes as an intended, covered decision rather than a defect.
  • Lane summary: The change replaces drop-on-overlap with extend-the-kept-hit in `dedupe_overlaps`, and adds a unit test that pins the new merging behaviour directly on the function. The test is real: with the old code `kept` would have held only `(0, 10, PII_CC)` and `assert_eq!(ranges, vec![(0, 15, PII_CC)])` would fail, so the invariant the new comment claims ("a partial overlap's tail is still covered") is pinned by a failing-if-false assertion. One gap worth noting but not reportable as a defect: the merge widens a `PII_CC` hit's range over bytes that were a `PII_PHONE` match, so the splice now emits `PII_CC`'s token for the whole span — the test asserts that token choice, so it is an intended, covered decision rather than a silent regression. _The code index is behind this pull request (indexed at `1f4681b009b7`), so retrieved context may be out of date._

commits

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The commits lane answered and reports no sensitive content in committed files.
  • Lane summary: No sensitive content in committed files. _The code index is behind this pull request (indexed at `1f4681b009b7`), so retrieved context may be out of date._

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The description lane answered, reports the change matches the diff, and found no defects in the changed code.
  • Lane summary: The change merges partially overlapping redaction hits into the kept hit instead of dropping the later one, so the combined range is redacted once under the first token; the added test pins the (0..10, 5..15) → (0..15) case. The description matches the diff and I found no defects in the changed code. _The code index is behind this pull request (indexed at `1f4681b009b7`), so retrieved context may be out of date._
Evidence and run details
  • Models: ladder/vectors-oai3, deepseek-v4.1-flash
  • Spend: $0.023015
  • Tokens: 126348 input · 3508 output · 31518 cached · 155 embedding
  • Updated: 2026-10-22T23:39:58Z
Head State Pass summary
528e35c14e5d ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-22T23:39:58Z)

tinysweeper 0.1.2

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tinysweeper found nothing blocking. Approving.

Diagnostics

Total spend: $0.0230

@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Oct 22, 2026
@M3gA-Mind
M3gA-Mind merged commit 58f36a0 into tinyhumansai:main Oct 23, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant