Skip to content

API design refresh: /v1/detect+sanitize -> /api/find+replace (0.5.0) - #20

Merged
jstjoe merged 5 commits into
mainfrom
jstjoe/api-reference-docs
May 13, 2026
Merged

jstjoe merged 5 commits into
mainfrom
jstjoe/api-reference-docs

Conversation

@jstjoe

@jstjoe jstjoe commented May 13, 2026

Copy link
Copy Markdown
Owner

Summary

Two breaking renames on top of #18/#19, in successive commits:

  1. Endpoint terminology/v1/detect -> /v1/find ("Find sensitive data") and /v1/sanitize -> /v1/replace ("Replace sensitive data"). Internal schemas, response field sanitized_text -> replaced_text, tags, internal helpers all follow.
  2. Drop URL versioning — paths move from /v1/* to /api/*. Pre-1.0; nobody outside this repo consumes the API yet. When the contract stabilizes the plan is header-based date-string versioning in the style of Stripe (API-Version: 2026-05-13).

info.version bumped 0.4.0 -> 0.5.0 across the two commits (one per breaking change).

New endpoint surface

Method Path Purpose
POST /api/find Find sensitive data. Returns spans, no rewriting.
POST /api/replace Find + replace each span under chosen mode.
GET /api/detectors List registered detector backends.
GET /api/health Liveness probe.

Old /v1/* paths return 404. No aliases.

Notable touches

Notebook (notebooks/pii_detector_comparison.ipynb) imports source and never hits HTTP — unaffected.

Versioning narrative (now in spec + overview guide)

The API is pre-1.0. info.version is the only version surface today. Each breaking change to request or response shape lands as a minor bump pre-1.0. URL paths stay unversioned (/api/...); future header API-Version: 2026-05-13 will pin clients to a release date once the contract is stable.

Test plan

  • uv run --package opf-api opf-api-export-openapi --out /tmp/fresh regenerates; diff against committed spec is empty
  • uvicorn opf_api.main:app boots; /api/health, /api/detectors, /scalar, /docs, /redoc, /openapi.json -> 200; /v1/* -> 404
  • pytest api/tests/test_routes.py — 21/21 passing (all rewritten for /api/*)
  • CI OpenAPI spec freshness job passes on latest push
  • After merge, Pages site at https://jstjoe.github.io/local-privacy/ reflects new endpoints

🤖 Generated with Claude Code

jstjoe and others added 2 commits May 13, 2026 08:38
…place

User-facing terminology landed for new consumers:

- Paths: `/v1/detect` -> `/v1/find` ("Find sensitive data");
  `/v1/sanitize` -> `/v1/replace` ("Replace sensitive data").
- Schemas: `DetectRequest`/`DetectResponse` -> `FindRequest`/`FindResponse`;
  `SanitizeRequest`/`SanitizeResponse` -> `ReplaceRequest`/`ReplaceResponse`;
  `SanitizedSpan` -> `ReplacedSpan`; `SanitizeMode` -> `ReplaceMode`.
- Response body: `sanitized_text` -> `replaced_text` (pairs naturally with
  the per-span `replacement` field).
- Tags: `Detect`/`Sanitize` -> `Find`/`Replace`.
- Internal: `_run_detect` -> `_run_find`, `_DetectInput` -> `_FindInput`,
  route handlers `detect`/`sanitize` -> `find`/`replace`.
- Bump `info.version` 0.3.0 -> 0.4.0 (breaking shape change pre-1.0).
- Rename `docs/guides/sanitize-modes.md` -> `replace-modes.md`; update
  README + every guide ref. Regenerate spec.

Old route names are gone — no aliases. Pre-1.0 contract; clients on the
old names need a one-line change.

Backend method `detector.detect()` (in opf_eval) keeps its name — that's
the in-process model API, not the public HTTP contract, and renaming it
spans an external package.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pre-1.0, nobody outside this repo consumes the API yet. URL-path
versioning is premature commitment; flip to unversioned `/api/...`
paths now and plan to add Stripe-style header-based date-string
versioning (`API-Version: 2026-05-13`) when the contract stabilizes.

- `app.include_router` prefix `/v1` -> `/api`.
- Bump `info.version` 0.4.0 -> 0.5.0 (breaking shape change).
- Update every route/schema docstring, description, response example
  reference, and curl snippet that mentioned `/v1/*`.
- README, every guide under `docs/guides/`, and
  `docs/token-vault-setup.md` re-pointed at `/api/*`.
- Update versioning section in main.py + overview guide to call out
  the planned `API-Version` header migration.
- Test bootstrap fixture also moved to `prefix="/api"`.

Notebook (`notebooks/pii_detector_comparison.ipynb`) imports source
directly and never hits the HTTP layer — unaffected by this change.

Old `/v1/*` paths return 404 — no aliases. Skyflow management API
paths (`/v1/vaults`, `/v1/workspaces`) in `docs/token-vault-setup.md`
are external endpoints and stay as-is.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jstjoe

jstjoe commented May 13, 2026

Copy link
Copy Markdown
Owner Author

@claude quick review please?

@claude

claude Bot commented May 13, 2026

Copy link
Copy Markdown

Claude finished @jstjoe's task in 2m 17s —— View job


Code Review

  • Read CLAUDE.md and gather context
  • Diff the PR changes
  • Review core API files (routes.py, schemas.py, main.py)
  • Review tests
  • Review docs/openapi spec
  • Post findings

Overall: LGTM. The rename is clean and thorough. No missed references on the API surface, tests all updated, spec regenerated, docs in sync. A few minor notes:

Stale comment in api/Dockerfile:53

# Bake OPF checkpoint into the image so first /v1/redact doesn't pay the

References the old /v1/redact path. Should be updated to /api/replace (or just drop the path reference entirely since the comment's substance is about eager-loading the checkpoint, not the route name). Fix this →

warning field is always None

Both find() (routes.py:198) and replace() (routes.py:312) hardcode warning=None. The field exists in FindResponse and ReplaceResponse with a description but is never populated. This predates this PR — just noting it as a cleanup opportunity whenever the field gets actual use or gets removed.

Overlapping span replacement on skipped spans

In replace(), detected_spans includes all spans (including ones whose text was skipped due to overlap in replaced_text). Each skipped span still has a populated replacement field even though that replacement was never spliced in. This is documented in the route description ("they still appear in detected_spans") and is pre-existing behaviour — but a client that trusts replacement as ground truth for what happened could be surprised. Worth documenting more explicitly in the field description eventually, or adding a replaced: bool flag to ReplacedSpan.

Everything else looks good

  • All /v1//api/ and sanitize/detectreplace/find renames are complete and consistent across source, tests, spec, and docs.
  • The only other /v1/ hits in the repo are Skyflow's own management API path in vault_tokens.py:129 and token-vault-setup.md — both correct and intentional.
  • ReplaceRequest correctly inherits from FindRequest; _FindInput Protocol is used for both handlers via _run_find.
  • Version 0.5.0 (jumping from 0.3.0 in the diff) is coherent: two successive minor bumps land in one PR, as the commit history shows.
  • docs/guides/sanitize-modes.md is gone; replace-modes.md exists with updated content; README and cross-references all point at the new name.

jstjoe and others added 2 commits May 13, 2026 09:07
`detected_spans` always carried every span the detector emitted, including
ones whose `replacement` was rendered but suppressed as a later-starting
overlap. The `replacement` field was populated on those spans too, so a
client reading them as ground truth would over-count what actually landed
in `replaced_text`. Documented in the route prose, but the field itself
made no distinction.

Add a required `replaced: bool` to `ReplacedSpan`:

- `replaced=true`  — span was spliced into `replaced_text`.
- `replaced=false` — span was dropped as a later-starting overlap of an
                     earlier-starting span. `replacement` is still
                     populated (the renderer ran) but the string did not
                     land.

Flag computed inline in `routes.py` using the same `start >= cursor`
walk that `splice_pieces` uses, so the two are guaranteed to agree.

`info.version` -> 0.6.0 (adding a required response field). Pre-1.0;
clients filter to `replaced=true` to reconstruct exactly what landed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The route name has churned (`/redact` -> `/sanitize` -> `/replace`) and
the comment's point is about eager-loading the 2.8 GB checkpoint, not the
endpoint. Reword to "first request" so the next rename doesn't strand the
comment again.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jstjoe

jstjoe commented May 13, 2026

Copy link
Copy Markdown
Owner Author

Thanks @claude, review again?

@claude

claude Bot commented May 13, 2026

Copy link
Copy Markdown

Claude finished @jstjoe's task in 1m 57s —— View job


Code Review (re-review)

  • Check new commits since last review
  • Review new changes in detail
  • Post findings

Both items flagged in the last review were shipped in follow-on commits — clean responses.

b18c276 — Dockerfile comment
Reworded to "first request" instead of patching the stale path. Better than just s/redact/replace — won't strand again on the next rename.

e77cc8creplaced: bool on ReplacedSpan
The implementation is correct and the cursor logic is a faithful mirror of splice_pieces:

  • routes.py checks s["start"] >= cursor and advances only on inclusion.
  • splice_pieces checks s["start"] < cursor to skip (semantically identical).

Both agree on the start == cursor edge case (keep the span). The flag is in the required list in both JSON and YAML specs, version bumped to 0.6.0, and the new test test_replace_overlapping_spans_flag_and_splice covers the two-span overlap scenario end-to-end.

One minor note on label_number numbering: when a span is overlapped and dropped, the renderer still ran for it (all spans in rendered_pairs get rendered before the flag walk). So a dropped span consumes a number — a later "Anderson" in the same request would get [PERSON_3] even though [PERSON_2] never appeared in replaced_text. Clients can infer this from replaced=false, but it may surprise people. Not a bug, just worth noting in a follow-up comment on the label_number row in the mode table if it ever becomes a support question.

The warning=None from last review is still hardcoded — but that was flagged as pre-existing, and is unchanged here.

Overall: LGTM. The rename is complete, the overlap transparency is solid, and the replaced flag gives clients a clean reconstruction path. Ready to merge.

The renderer runs for every span before overlap suppression, so a span
that ends up `replaced=false` still consumes its label's counter. Kept
spans on the same label after a dropped overlap therefore skip numbers
(e.g. `[PERSON_1]` then `[PERSON_3]`, gap = the dropped span). Not a
bug — a consequence of doing the splice walk after rendering — but
worth flagging where someone would actually look:

- `/api/replace` route description: mode table row for `label_number`
  now points at the new overlap-notes paragraph.
- `docs/guides/replace-modes.md`: same table-row hint plus a dedicated
  paragraph in the Overlapping-spans section explaining the gap.

No behavior change; spec regenerated to pick up the inline description.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jstjoe
jstjoe merged commit 527cdf9 into main May 13, 2026
4 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