Skip to content

chore: replace hand-rolled compliance-pack cache with functools.cache - #117

Merged
JohnnyWilson16 merged 1 commit into
mainfrom
chore/ponytail-audit-cuts
Jul 10, 2026
Merged

chore: replace hand-rolled compliance-pack cache with functools.cache#117
JohnnyWilson16 merged 1 commit into
mainfrom
chore/ponytail-audit-cuts

Conversation

@JohnnyWilson16

Copy link
Copy Markdown
Contributor

Summary

  • Ran a ponytail over-engineering audit against the repo. Most flagged items didn't survive closer investigation (see below) — this PR is the one cut that held up.
  • _PACK_CACHE in privacy_policy.py was a hand-rolled dict memo for compliance-pack loading. Replaced with @functools.cache on a small _load_builtin_pack helper. Behavior is unchanged: only built-in pack names (hipaa/ferpa/pci/gdpr) are cached; arbitrary file/path loads are still re-read every call, as before.

Audit items investigated and explicitly not included

  • seed param on simple.py outlier functions — looks unused, but it's a documented, public, tested API contract (tests/test_simple.py:119-120). Removing it is a breaking change out of scope here.
  • Duplicate-looking edit-distance functions (entity_resolution.py::levenshtein vs semantic/formats.py::_edit_distance) — not actually duplicates; the second is a banded/capped fast-path used in a hot loop. Merging would regress performance.
  • INTENT_ID / fd-intent-v1 registry entry — looked like dead forward-compat cruft, but there's a full dev-time training pipeline (training/distill/train_intent_head.py), a model card, and docs targeting this exact artifact. Real in-progress infra, not dead code.
  • Three benchmark trees (benchmarks/, freshdata-benchmarks/, src/freshdata/benchmarks/) — initially flagged as redundant, but each serves a distinct purpose (CI-gated regression suite, competitor-comparison ASV suite last touched 5 days ago, and the shipped out-of-core-engine benchmark referenced by docs/claims). Not consolidating.
  • FreshCore Rust backend (crates/freshcore/) — flagged as unpublished/uninstallable bloat, but it's one-week-old active work with its own docs page, README mention, and dedicated tests. Confirmed with the repo owner to keep it rather than delete based on a heuristic that assumed it was stale.

Test plan

  • pytest tests/test_privacy_policy.py tests/test_compliance — pass
  • Full suite (pytest tests -q --no-cov, excluding the local-only tests/benchmark) — pass
  • ruff check — clean

_PACK_CACHE was a manual dict memo duplicating what the stdlib already
does; swap it for @functools.cache on the built-in-pack loader. External
file/path loads (the other branch of load_compliance_pack) are
intentionally left uncached, matching prior behavior.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@strix-security

strix-security Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Strix Security Review

No security issues found.

Updated for d83baae.


Reviewed by Strix
Re-run review · Configure security review settings

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@JohnnyWilson16, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 10 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ed014964-c7ec-46b3-a719-22b7af53983e

📥 Commits

Reviewing files that changed from the base of the PR and between 966cb49 and d83baae.

📒 Files selected for processing (1)
  • src/freshdata/enterprise/privacy_policy.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/ponytail-audit-cuts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions

Copy link
Copy Markdown

FreshData benchmark report — 2026-07-10T16:43:58Z

  • freshdata: 1.1.1
  • python: 3.12.13
  • platform: Linux-6.17.0-1018-azure-x86_64-with-glibc2.39
fixture n_rows n_cols p50 s p95 s peak MB repair % false-repair % preserve % trust monotonic export %
crm 10,200 40 1.325 1.334 14.0 100.0 0.0 100.0 93.84 100.0
event_log 10,000 25 0.498 0.498 5.1 100.0 0.0 100.0 99.677 100.0
finance 10,200 60 1.690 1.691 18.3 100.0 0.0 100.0 99.499 100.0
gold 10,200 7 0.193 0.195 3.7 100.0 0.0 100.0 98.3 100.0
provenance 10,000 18 0.412 0.413 7.2 100.0 0.0 100.0 99.765 100.0
wide_schema 10,000 100 3.109 3.113 13.4 100.0 0.0 100.0 96.042 100.0

Authored-code reduction (Metric 6)

  • FreshData: 3 lines
  • pandas baseline: 26 lines (88.46% reduction)

@JohnnyWilson16
JohnnyWilson16 merged commit 22fb3cd into main Jul 10, 2026
22 of 23 checks passed
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.

1 participant