Jurisdiction#79
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 31 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
WalkthroughThe PR replaces request-level detection jurisdictions with Presidio sidecar load-time country scoping, removes related gateway, storage, engine, protocol, and UI plumbing, broadens internal header stripping, updates verification coverage, and derives development build IDs from Git when unset. ChangesPresidio country-scoped registry
Gateway jurisdiction contract removal
Engine and proxy profile removal
Development build metadata
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Gateway
participant FictaProxy
participant ProtectionEngine
participant Upstream
Client->>Gateway: Send chat or protection-preview request
Gateway->>FictaProxy: Forward scope and request data without detection profile
FictaProxy->>ProtectionEngine: Begin request by scope key
ProtectionEngine->>Upstream: Forward request after removing x-ficta-* headers
Upstream-->>ProtectionEngine: Return response
ProtectionEngine-->>Gateway: Return protected response or ticket result
Gateway-->>Client: Return chat or preview response
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/supported-countries-registry.md:
- Around line 10-12: Remove internal evaluation details from
.changeset/supported-countries-registry.md lines 10-12, including the
false-positive measurement and example, while retaining only the default
behavior and UK opt-in instructions. Also update
packages/ficta/presidio/default_recognizers.yaml lines 20-24 to remove “retained
for parity” and the unresolved “revisit” note, documenting only the effective
default behavior.
In `@packages/ficta/presidio/ficta_presidio/service.py`:
- Around line 29-34: Update the FICTA_PRESIDIO_SUPPORTED_COUNTRIES handling to
normalize requested ISO-3166-1 alpha-2 codes, validate every code against the
registry’s supported country codes, and raise before assigning
registry.recognizers when any are unknown. Preserve the existing filtering
behavior and ensure an explicitly empty list continues to select only
locale-agnostic recognizers.
In `@scripts/dev-env.mjs`:
- Around line 166-168: Update the status handling in the dev identifier logic to
return undefined when spawnSync for “git status --porcelain” reports a non-zero
exit status, before constructing the dev-${sha} value. Preserve the existing
dirty-tree suffix behavior only for successful status checks.
In `@scripts/dev-runner.mjs`:
- Around line 125-128: Update the existing analyzer reuse and health-check flow
in the dev runner, not only the container spawn arguments, so an explicitly
configured FICTA_PRESIDIO_SUPPORTED_COUNTRIES scope is verified against the
running sidecar’s loaded recognizers before reuse. If the scope cannot be
confirmed or does not match, reject the existing sidecar and continue with
fresh-container startup; preserve reuse only when the country scope is
validated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 13cec79e-40e0-4cef-adec-98d446ba2da3
📒 Files selected for processing (47)
.changeset/supported-countries-registry.md.changeset/thread-jurisdiction-detection.mdapps/gateway/.env.exampleapps/gateway/README.mdapps/gateway/docs/threat-model-pii.mdapps/gateway/drizzle/0013_drop-detection-jurisdictions.sqlapps/gateway/drizzle/meta/0013_snapshot.jsonapps/gateway/drizzle/meta/_journal.jsonapps/gateway/src/components/chat/ChatView.tsxapps/gateway/src/components/chat/JurisdictionSidebar.tsxapps/gateway/src/components/chat/TopBar.tsxapps/gateway/src/lib/detection-jurisdictions.tsapps/gateway/src/lib/model-adapter.tsapps/gateway/src/lib/protection-preview.tsapps/gateway/src/lib/storage/drizzle/schema.tsapps/gateway/src/lib/storage/drizzle/store.server.tsapps/gateway/src/lib/storage/storage.server.tsapps/gateway/src/lib/storage/threads.tsapps/gateway/src/lib/storage/types.tsapps/gateway/src/routes/api/chat.tsapps/gateway/src/routes/api/protection-preview.tsapps/gateway/src/routes/chat.$threadId.tsxapps/gateway/test/detection-jurisdictions.test.tsapps/gateway/test/storage.test.tsdocker-compose.sidecars.ymlpackages/ficta/README.mdpackages/ficta/docs/plugins.mdpackages/ficta/presidio/Dockerfilepackages/ficta/presidio/default_recognizers.yamlpackages/ficta/presidio/ficta_presidio/service.pypackages/ficta/scripts/verify-presidio-sidecar.mtspackages/ficta/src/engine/engine.tspackages/ficta/src/engine/plugins/pii/jurisdictions.tspackages/ficta/src/engine/plugins/pii/presidio-recognizer.tspackages/ficta/src/engine/plugins/types.tspackages/ficta/src/engine/redaction-engine.tspackages/ficta/src/server.tspackages/ficta/test/dev-runner.test.tspackages/ficta/test/jurisdictions.test.tspackages/ficta/test/loopback-multiturn.test.tspackages/ficta/test/presidio.test.tspackages/ficta/test/protection-preview.test.tspackages/ficta/test/scope.test.tspackages/protocol/src/index.d.tspackages/protocol/src/index.jsscripts/dev-env.mjsscripts/dev-runner.mjs
💤 Files with no reviewable changes (21)
- apps/gateway/src/lib/detection-jurisdictions.ts
- apps/gateway/src/lib/protection-preview.ts
- apps/gateway/test/detection-jurisdictions.test.ts
- apps/gateway/src/lib/storage/types.ts
- packages/ficta/src/engine/plugins/pii/jurisdictions.ts
- packages/ficta/src/engine/plugins/types.ts
- packages/protocol/src/index.js
- apps/gateway/src/lib/storage/drizzle/schema.ts
- apps/gateway/src/routes/chat.$threadId.tsx
- .changeset/thread-jurisdiction-detection.md
- apps/gateway/src/components/chat/JurisdictionSidebar.tsx
- apps/gateway/src/lib/storage/storage.server.ts
- packages/ficta/test/jurisdictions.test.ts
- apps/gateway/src/lib/model-adapter.ts
- apps/gateway/src/lib/storage/threads.ts
- apps/gateway/src/routes/api/protection-preview.ts
- apps/gateway/src/components/chat/TopBar.tsx
- apps/gateway/test/storage.test.ts
- packages/ficta/test/protection-preview.test.ts
- packages/protocol/src/index.d.ts
- packages/ficta/test/scope.test.ts
| locale-agnostic recognizers always load. Notably, the UK NHS recognizer no longer runs on default | ||
| traffic (it false-positives on ~10% of arbitrary 10-digit numbers, e.g. ZA phone and account | ||
| numbers) — add `uk` to the country scope for UK-matter deployments. `FICTA_PII_PRESIDIO_ENTITIES` |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Keep internal evaluation and strategy out of published package content.
Both sites disclose internal rationale rather than only the supported public contract:
.changeset/supported-countries-registry.md#L10-L12: remove the false-positive measurement and example; retain only the default behavior and opt-in instructions.packages/ficta/presidio/default_recognizers.yaml#L20-L24: remove “retained for parity” and the unresolved “revisit” note; document only the effective default.
As per coding guidelines, red-team findings, internal specifications, and unreleased strategy must remain in SerovaAI/ficta-internal.
📍 Affects 2 files
.changeset/supported-countries-registry.md#L10-L12(this comment)packages/ficta/presidio/default_recognizers.yaml#L20-L24
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.changeset/supported-countries-registry.md around lines 10 - 12, Remove
internal evaluation details from .changeset/supported-countries-registry.md
lines 10-12, including the false-positive measurement and example, while
retaining only the default behavior and UK opt-in instructions. Also update
packages/ficta/presidio/default_recognizers.yaml lines 20-24 to remove “retained
for parity” and the unresolved “revisit” note, documenting only the effective
default behavior.
Source: Coding guidelines
Summary by CodeRabbit
New Features
Changes
Documentation