feat: pii_core + redact-proxy MVP (non-streaming) #1

Merged

jonah merged 2 commits from feat/core-and-redact-proxy into main 2026-10-05 21:02:07 +02:00

Conversation 5 Commits 2 Files changed 40 +2636 -1

jonah commented 2026-10-04 13:04:56 +02:00

Owner

Copy link

Build steps 1-2 of BRIEF.md.

What

• packages/core (pii_core): checksum validators + Presidio recognizers for NL/DE/common PII.
• apps/redact-proxy: OpenAI-compatible /v1/chat/completions that redacts, forwards, restores. Dry-run, counts-only audit log, YAML config, Docker.

Verified

• uv run pytest -m "not integration": 70 passed. -m integration (real nl_core_news_md NER + context scoring): passed.
• pre-commit run --all-files (ruff, format, import-linter, gitleaks, ...): passed.
• Manual dry-run against the running server redacted name, email, BSN, KvK (via context), IBAN, city; allow-listed company kept.

Not verified

• docker compose up --build: Docker daemon was not running. Dockerfile and compose are untested.
• CI on Forgejo has not run yet.
• Live round trip through Ollama (only dry-run and mock-upstream tests).

Known gaps

Streaming, tool-call arguments, per-request language detection, accuracy table, bare NL postcode (score 0.3 < threshold). See CHANGELOG.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JZCbAYmY516SAe7umoLKWt

Build steps 1-2 of BRIEF.md. ## What - `packages/core` (`pii_core`): checksum validators + Presidio recognizers for NL/DE/common PII. - `apps/redact-proxy`: OpenAI-compatible `/v1/chat/completions` that redacts, forwards, restores. Dry-run, counts-only audit log, YAML config, Docker. ## Verified - `uv run pytest -m "not integration"`: 70 passed. `-m integration` (real `nl_core_news_md` NER + context scoring): passed. - `pre-commit run --all-files` (ruff, format, import-linter, gitleaks, ...): passed. - Manual dry-run against the running server redacted name, email, BSN, KvK (via context), IBAN, city; allow-listed company kept. ## Not verified - `docker compose up --build`: Docker daemon was not running. Dockerfile and compose are untested. - CI on Forgejo has not run yet. - Live round trip through Ollama (only dry-run and mock-upstream tests). ## Known gaps Streaming, tool-call arguments, per-request language detection, accuracy table, bare NL postcode (score 0.3 < threshold). See CHANGELOG. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JZCbAYmY516SAe7umoLKWt

jonah added 1 commit 2026-10-04 13:05:15 +02:00

feat: pii_core validators/recognizers and non-streaming redact-proxy MVP ... d684b359ee

- pii_core: BSN, IBAN, Luhn, KvK, NL BTW, DE Steuer-ID/USt-IdNr validators;
  Presidio recognizers for NL/DE/common via build_registry
- redact-proxy: /v1/chat/completions with per-request in-memory mapping,
  dry-run mode, counts-only audit log, YAML config, /v1/models passthrough
- Docker + compose, uv workspace, import-linter contracts, CI wiring
- 70 hermetic tests + 1 integration test (real spaCy NER)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JZCbAYmY516SAe7umoLKWt

jonah referenced this pull request 2026-10-05 14:03:10 +02:00

feat: accuracy harness + synthetic test set (stacked on #1) #2

jonah commented 2026-10-05 17:03:00 +02:00

Author

Owner

Copy link

🤖 Auto-review status · jonah/safe-ai-tools PR #1

📋 Review posted — fixes required, awaiting human action.

Last updated: 2026-10-05T15:38:01.211Z

<!-- opencode-status --> **🤖 Auto-review status** · `jonah/safe-ai-tools` PR #1 📋 **Review posted** — fixes required, awaiting human action. *Last updated: 2026-10-05T15:38:01.211Z*

jonah reviewed 2026-10-05 17:38:01 +02:00

jonah left a comment

Copy link

[STATUS: FIXES_REQUIRED]

Findings

Required fixes

• apps/redact-proxy/src/redact_proxy/config.py + analyzer.py — configured entities are never validated (fail-open). Config.entities accepts arbitrary strings. Verified against installed presidio: a typo'd entity mixed into a valid list (["PERSON", "NL_VBS"]) is silently ignored — under-redaction, the exact fail-open direction (AGENTS.md XIX — "deny rather than grant"); a list containing only unknown names raises a raw ValueError("No matching recognizers were found to serve the request.") from the recognizer registry inside the request thread (unhandled 500). Fix in build_analyze (apps/redact-proxy/src/redact_proxy/analyzer.py:29): after building the engine, assert every config.entities entry is in the engine's supported entities for the language and raise a clear startup error otherwise. Cheap, and it converts a silent privacy regression into a fail-fast.

• README.md:39 — repo toolchain is red. ruff format --check . reports README.md would be reformatted (missing blank line after from openai import OpenAI in the ```python block). CI's checks job runs pre-commit ruff-format; the dev-group ruff (0.16.10, resolves via uv.lock) flags it either way. One blank line.

Minor / non-blocking

• apps/redact-proxy/src/redact_proxy/analyzer.py:22 — a user-supplied partial spacy_models dict that omits config.language yields a bare KeyError at startup instead of a pydantic validation error. Add a model-level validator so config errors are all one kind.
• Placeholder collision (inherent to the token format). If a user prompt contains a literal [LABEL_n] that later collides with a mapped placeholder, restore substitutes the real PII value for the user's literal in the reply — round-trip distortion, not a leak. Worth a line in the README limitations rather than a code change.
• Redaction scope: only content strings and text-type content parts are redacted; tool_calls[].function.arguments, message name, and response-side non-message.content fields pass through untouched. Consistent with the MVP docstring/README "non-streaming MVP", but the tool_calls args path is a genuine PII gap the README doesn't state — add it to the limitations.
• analyzer.py 47% coverage; CI runs -m "not integration", so the NER wiring is only exercised via the skip-guarded integration test — fine for now (hermetic-by-default is the right call), worth a milestone ticket.
• apps/redact-proxy/tests/conftest.py:18 uses rec.analyze(text, rec.supported_entities) without conflict resolution — fine for hermetic tests, but note it diverges from production dedup behavior; the app-level tests cover the composed path.
• Uncovered lines app.py:139 (healthz body), app.py:147-148 (models 502 path), upstream.py:23, config.py:59 — trivial, no gate.

File/function discipline, naming, error contracts, layering, secret hygiene, and test determinism are all well above the bar; the two required fixes are both small and local.

[STATUS: FIXES_REQUIRED] ## Findings ### Required fixes - **`apps/redact-proxy/src/redact_proxy/config.py` + `analyzer.py` — configured entities are never validated (fail-open).** `Config.entities` accepts arbitrary strings. Verified against installed presidio: a typo'd entity mixed into a valid list (`["PERSON", "NL_VBS"]`) is **silently ignored** — under-redaction, the exact fail-open direction (`AGENTS.md` XIX — "deny rather than grant"); a list containing *only* unknown names raises a raw `ValueError("No matching recognizers were found to serve the request.")` from the recognizer registry inside the request thread (unhandled 500). Fix in `build_analyze` (`apps/redact-proxy/src/redact_proxy/analyzer.py:29`): after building the engine, assert every `config.entities` entry is in the engine's supported entities for the language and raise a clear startup error otherwise. Cheap, and it converts a silent privacy regression into a fail-fast. - **`README.md:39` — repo toolchain is red.** `ruff format --check .` reports README.md would be reformatted (missing blank line after `from openai import OpenAI` in the ```python block). CI's `checks` job runs pre-commit ruff-format; the dev-group ruff (0.16.10, resolves via `uv.lock`) flags it either way. One blank line. ### Minor / non-blocking - **`apps/redact-proxy/src/redact_proxy/analyzer.py:22`** — a user-supplied partial `spacy_models` dict that omits `config.language` yields a bare `KeyError` at startup instead of a pydantic validation error. Add a model-level validator so config errors are all one kind. - **Placeholder collision (inherent to the token format).** If a user prompt contains a literal `[LABEL_n]` that later collides with a mapped placeholder, `restore` substitutes the real PII value for the user's literal in the reply — round-trip distortion, not a leak. Worth a line in the README limitations rather than a code change. - **Redaction scope:** only `content` strings and text-type content parts are redacted; `tool_calls[].function.arguments`, message `name`, and response-side non-`message.content` fields pass through untouched. Consistent with the MVP docstring/README "non-streaming MVP", but the `tool_calls` args path is a genuine PII gap the README doesn't state — add it to the limitations. - **`analyzer.py` 47% coverage; CI runs `-m "not integration"`,** so the NER wiring is only exercised via the skip-guarded integration test — fine for now (hermetic-by-default is the right call), worth a milestone ticket. - **`apps/redact-proxy/tests/conftest.py:18`** uses `rec.analyze(text, rec.supported_entities)` without conflict resolution — fine for hermetic tests, but note it diverges from production dedup behavior; the app-level tests cover the composed path. - Uncovered lines `app.py:139` (healthz body), `app.py:147-148` (models 502 path), `upstream.py:23`, `config.py:59` — trivial, no gate. File/function discipline, naming, error contracts, layering, secret hygiene, and test determinism are all well above the bar; the two required fixes are both small and local.

jonah added 1 commit 2026-10-05 20:34:53 +02:00

fix: validate configured entities and spacy_models at startup ... 860c700db8

Unknown entity names were silently ignored (under-redaction) or caused a
500 per request. Fail fast instead. Also fix README format and document
placeholder collision. Bump to 0.1.1.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

jonah commented 2026-10-05 20:36:57 +02:00

Author

Owner

Copy link

🤖 Auto-review status · jonah/safe-ai-tools PR #1

✅ PR is clean — no fixes required.

Last updated: 2026-10-05T18:43:59.729Z

<!-- opencode-status --> **🤖 Auto-review status** · `jonah/safe-ai-tools` PR #1 ✅ **PR is clean** — no fixes required. *Last updated: 2026-10-05T18:43:59.729Z*

jonah commented 2026-10-05 20:38:09 +02:00

Author

Owner

Copy link

Addressed in 860c700 (v0.1.1):

• Fixed: unknown entities now raise a clear ValueError at startup in build_analyze, before any model load (test: test_config.py).
• Fixed: README python block formatting.
• Fixed: spacy_models missing the configured language is now a pydantic validation error.
• Docs: placeholder-collision limitation added to README.
• Skipped: tool_calls args (already listed in README limits); coverage/conftest/trivial uncovered lines (non-gating).
  PR #2 rebased onto this.

Addressed in 860c700 (v0.1.1): - Fixed: unknown `entities` now raise a clear ValueError at startup in `build_analyze`, before any model load (test: `test_config.py`). - Fixed: README python block formatting. - Fixed: `spacy_models` missing the configured language is now a pydantic validation error. - Docs: placeholder-collision limitation added to README. - Skipped: tool_calls args (already listed in README limits); coverage/conftest/trivial uncovered lines (non-gating). PR #2 rebased onto this.

jonah reviewed 2026-10-05 20:43:59 +02:00

jonah left a comment

Copy link

[STATUS: CLEAN]

Both previously-required fixes are implemented and verified; no new issues found. One non-blocking observation below.

Verification of prior required fixes

• Entity validation (fail-fast) — fixed. apps/redact-proxy/src/redact_proxy/analyzer.py:24-28 now builds the registry first and raises ValueError(f"Unknown entities for '{language}': ...") for any config.entities entry the registry doesn't serve, before NlpEngineProvider loads a model. Confirmed against installed presidio: RecognizerRegistry.get_supported_entities(languages=[...]) exists with that signature, filters recognizers by language, and the default entity set validates clean for nl, de, and en. Since __main__.py:21 calls build_analyze(config) before uvicorn.run, this is a true startup error, not a per-request 500.
• README toolchain red — fixed. README.md:38 blank line added. Verified uv run ruff format --check . passes — ruff 0.16.10 does format python blocks inside markdown (confirmed with a temp file), so the gate was real and is now green.
• Prior minor items addressed: spacy_models now a pydantic model_validator error (config.py:51-55), placeholder-collision limitation documented (README.md:69), and the tool_calls limitation was already present (README.md:67). New tests/test_config.py covers both paths (100%); analyzer.py coverage 47% → 72%.

Checks run (all green)

• pytest -m "not integration": 72 passed; pytest -m integration: 1 passed (real spaCy model through the reordered build_analyze)
• ruff check . / ruff format --check .: clean · lint-imports: 2 contracts kept
• SKIP=import-linter pre-commit run --all-files (CI checks job equivalent): all hooks pass
• uv lock --check: lockfile fresh · VERSION + CHANGELOG.md both bumped, satisfying the CI policy job · working tree clean

Minor / non-blocking

• apps/redact-proxy/src/redact_proxy/analyzer.py:21 — docstring claims OSError: the configured spaCy model is not installed, but presidio's SpacyNlpEngine/SlimSpacyNlpEngine auto-download missing models via spacy.cli.download instead (observed firsthand: a 541 MB fetch when build_analyze ran without a model). Two consequences: the documented OSError is effectively dead, and the README's "the only outbound call is the one to the upstream you configure" is untrue if the operator skips the model-install step. Docker pre-installs models, so this is a bare-metal/docs nit only — a one-line docstring correction, or pin auto_download=False if strict offline startup matters.

[STATUS: CLEAN] Both previously-required fixes are implemented and verified; no new issues found. One non-blocking observation below. ## Verification of prior required fixes - **Entity validation (fail-fast) — fixed.** `apps/redact-proxy/src/redact_proxy/analyzer.py:24-28` now builds the registry first and raises `ValueError(f"Unknown entities for '{language}': ...")` for any `config.entities` entry the registry doesn't serve, *before* `NlpEngineProvider` loads a model. Confirmed against installed presidio: `RecognizerRegistry.get_supported_entities(languages=[...])` exists with that signature, filters recognizers by language, and the default entity set validates clean for `nl`, `de`, and `en`. Since `__main__.py:21` calls `build_analyze(config)` before `uvicorn.run`, this is a true startup error, not a per-request 500. - **README toolchain red — fixed.** `README.md:38` blank line added. Verified `uv run ruff format --check .` passes — ruff 0.16.10 does format python blocks inside markdown (confirmed with a temp file), so the gate was real and is now green. - **Prior minor items addressed:** `spacy_models` now a pydantic `model_validator` error (`config.py:51-55`), placeholder-collision limitation documented (`README.md:69`), and the `tool_calls` limitation was already present (`README.md:67`). New `tests/test_config.py` covers both paths (100%); `analyzer.py` coverage 47% → 72%. ## Checks run (all green) - `pytest -m "not integration"`: 72 passed; `pytest -m integration`: 1 passed (real spaCy model through the reordered `build_analyze`) - `ruff check .` / `ruff format --check .`: clean · `lint-imports`: 2 contracts kept - `SKIP=import-linter pre-commit run --all-files` (CI `checks` job equivalent): all hooks pass - `uv lock --check`: lockfile fresh · `VERSION` + `CHANGELOG.md` both bumped, satisfying the CI `policy` job · working tree clean ## Minor / non-blocking - **`apps/redact-proxy/src/redact_proxy/analyzer.py:21`** — docstring claims `OSError: the configured spaCy model is not installed`, but presidio's `SpacyNlpEngine`/`SlimSpacyNlpEngine` auto-download missing models via `spacy.cli.download` instead (observed firsthand: a 541 MB fetch when `build_analyze` ran without a model). Two consequences: the documented `OSError` is effectively dead, and the README's "the only outbound call is the one to the upstream you configure" is untrue if the operator skips the model-install step. Docker pre-installs models, so this is a bare-metal/docs nit only — a one-line docstring correction, or pin `auto_download=False` if strict offline startup matters.

jonah merged commit 3f72a2fcbe into main 2026-10-05 21:02:07 +02:00

jonah referenced this pull request from a commit 2026-10-05 21:02:07 +02:00

Merge pull request 'feat: pii_core + redact-proxy MVP (non-streaming)' (#1) from feat/core-and-redact-proxy into main

jonah referenced this pull request from a commit 2026-10-05 21:14:49 +02:00

feat: accuracy harness + synthetic test set (stacked on #1) (#2)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

jonah/safe-ai-tools!1

No description provided.