Skip to content

Commit 86cf2f3

Browse files
Preserve configuration fallbacks and refund superseded extraction attempts
1 parent bc6176f commit 86cf2f3

5 files changed

Lines changed: 145 additions & 17 deletions

File tree

‎efile_app/efile/api/case_type_config.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ def get_case_type_config(request):
1111
jurisdiction = request.GET.get("jurisdiction") or request.session.get("jurisdiction")
1212

1313
# Use the new jurisdiction-aware configuration loader
14-
config_data = config_loader.load_jurisdiction_config(jurisdiction)
14+
config_data = (
15+
config_loader.base_config if jurisdiction is None else config_loader.load_jurisdiction_config(jurisdiction)
16+
)
1517

1618
# Process case types to ensure proper inheritance from base_case_types
1719
processed_case_types = {}

‎efile_app/efile/context_processors.py‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
Context processors for jurisdiction-aware templates
33
"""
44

5-
from .utils.config_loader import config_loader
5+
from .utils.config_loader import InvalidJurisdiction, config_loader
66

77

88
def jurisdiction_context(request):
@@ -21,7 +21,12 @@ def jurisdiction_context(request):
2121

2222
# Generic pages have no selected state. Do not pass absence through the
2323
# strict request-to-configuration boundary as a jurisdiction identifier.
24-
config = config_loader.load_jurisdiction_config(current_jurisdiction) if current_jurisdiction else {}
24+
try:
25+
config = config_loader.load_jurisdiction_config(current_jurisdiction) if current_jurisdiction else {}
26+
except InvalidJurisdiction:
27+
# Error templates also run context processors. Invalid request input
28+
# must not prevent Django from rendering the original error response.
29+
config = {}
2530

2631
return {
2732
"jurisdiction": current_jurisdiction,

‎efile_app/efile/services/document_extractions.py‎

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
from django.conf import settings
1313
from django.db import connection, transaction
14-
from django.db.models import Q
14+
from django.db.models import F, Q
1515
from django.utils import timezone
1616
from markitdown import MarkItDown
1717
from pypdf import PdfReader, PdfWriter
@@ -38,6 +38,10 @@
3838
logger = logging.getLogger(__name__)
3939

4040

41+
class ExtractionSuperseded(Exception):
42+
"""The requested analysis changed; this is not a processing failure."""
43+
44+
4145
def queue_document_extraction(document):
4246
"""Create or reset the one background extraction job for a lead PDF."""
4347
if document.role != FilingDocument.Role.LEAD:
@@ -326,14 +330,18 @@ def check_outbound_permission():
326330
)
327331
.exists()
328332
):
329-
raise RuntimeError("Extraction claim superseded")
330-
331-
analysis = analyze_document(
332-
analysis_path,
333-
document.draft.jurisdiction,
334-
use_ai=not opted_out,
335-
before_outbound=check_outbound_permission,
336-
)
333+
raise ExtractionSuperseded
334+
335+
try:
336+
analysis = analyze_document(
337+
analysis_path,
338+
document.draft.jurisdiction,
339+
use_ai=not opted_out,
340+
before_outbound=check_outbound_permission,
341+
)
342+
except ExtractionSuperseded:
343+
_requeue_changed_preference(job_id, claim_token, opted_out)
344+
return None
337345

338346
# Keep compatibility with extensions that still return the old flat shape.
339347
if "guesses" in analysis and isinstance(analysis.get("guesses"), dict):
@@ -353,7 +361,10 @@ def check_outbound_permission():
353361
job = _current_claim(job_id, claim_token).select_for_update().first()
354362
if job is None:
355363
return None
356-
if draft is None or draft.ai_assistance_opted_out != opted_out:
364+
if draft is None:
365+
return None
366+
if draft.ai_assistance_opted_out != opted_out:
367+
_requeue_changed_preference(job_id, claim_token, opted_out)
357368
return None
358369
document = job.document
359370
# A filer can remove or replace the lead while this worker is running.
@@ -455,6 +466,29 @@ def renew_extraction_lease(job_id, claim_token):
455466
return bool(_current_claim(job_id, claim_token).update(lease_expires_at=timezone.now() + timedelta(minutes=15)))
456467

457468

469+
def _requeue_changed_preference(job_id, claim_token, opted_out):
470+
"""Refund a superseded attempt without resetting earlier real failures."""
471+
now = timezone.now()
472+
changed_documents = FilingDocument.objects.filter(draft__ai_assistance_opted_out=not opted_out).values("pk")
473+
# Keep the claim predicates on the UPDATE itself, rather than inside a
474+
# joined-query subselect, so a concurrent new claim remains protected.
475+
return (
476+
_current_claim(job_id, claim_token)
477+
.filter(document_id__in=changed_documents)
478+
.update(
479+
status=DocumentExtraction.Status.PENDING,
480+
attempts=F("attempts") - 1,
481+
claim_token=None,
482+
lease_expires_at=None,
483+
available_at=now,
484+
started_at=None,
485+
completed_at=None,
486+
error="",
487+
updated_at=now,
488+
)
489+
)
490+
491+
458492
def record_extraction_failure(job_id, claim_token, error):
459493
"""Retry transient failures, then expose a manual-entry fallback."""
460494
with transaction.atomic():

‎efile_app/efile/tests/test_extraction_claims.py‎

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,14 +99,16 @@ def download(*args):
9999
analyze.assert_not_called()
100100

101101

102-
def test_preference_change_during_local_conversion_stops_model_request(lead):
102+
@pytest.mark.parametrize("requeue", [True, False])
103+
def test_preference_change_during_local_conversion_stops_model_request(lead, requeue):
103104
queue_document_extraction(lead)
104105
job = claim_next_extraction()
105106

106107
def convert(*args):
107108
lead.draft.ai_assistance_opted_out = True
108109
lead.draft.save()
109-
queue_document_extraction(lead)
110+
if requeue:
111+
queue_document_extraction(lead)
110112
return "local text", 1
111113

112114
with (
@@ -120,9 +122,53 @@ def convert(*args):
120122
patch("efile.services.document_extractions.get_default_model", return_value="test-model"),
121123
patch("efile.services.document_extractions.extract_fields_from_file") as extract,
122124
):
123-
with pytest.raises(RuntimeError, match="superseded"):
124-
process_document_extraction(job.pk, job.claim_token)
125+
assert process_document_extraction(job.pk, job.claim_token) is None
125126
extract.assert_not_called()
127+
assert record_extraction_failure(job.pk, job.claim_token, "Worker exited") is None
128+
job.refresh_from_db()
129+
assert job.status == DocumentExtraction.Status.PENDING
130+
assert job.attempts == 0
131+
assert job.error == ""
132+
assert claim_next_extraction() is not None
133+
134+
135+
@pytest.mark.parametrize("attempts_before", [0, 2])
136+
@pytest.mark.parametrize("initial_opted_out", [True, False])
137+
@override_settings(DOCUMENT_EXTRACTION_MAX_ATTEMPTS=3)
138+
def test_preference_change_at_completion_refunds_only_obsolete_attempt(lead, attempts_before, initial_opted_out):
139+
FilingDraft.objects.filter(pk=lead.draft_id).update(ai_assistance_opted_out=initial_opted_out)
140+
job = queue_document_extraction(lead)
141+
DocumentExtraction.objects.filter(pk=job.pk).update(attempts=attempts_before)
142+
claimed = claim_next_extraction()
143+
144+
def analyze(*args, **kwargs):
145+
# Simulate an update that does not use the normal requeue endpoint.
146+
FilingDraft.objects.filter(pk=lead.draft_id).update(ai_assistance_opted_out=not initial_opted_out)
147+
return {"document title": "obsolete result"}
148+
149+
with (
150+
patch(
151+
"efile.services.document_extractions.S3UploadHandler",
152+
return_value=Mock(download_file=Mock(return_value={"success": True})),
153+
),
154+
patch("efile.services.document_extractions.limited_pdf", fake_pdf),
155+
patch("efile.services.document_extractions.analyze_document", side_effect=analyze),
156+
):
157+
assert process_document_extraction(job.pk, claimed.claim_token) is None
158+
# The supervisor must treat the child's normal exit as a no-op.
159+
assert record_extraction_failure(job.pk, claimed.claim_token, "Worker exited") is None
160+
job.refresh_from_db()
161+
assert job.status == DocumentExtraction.Status.PENDING
162+
assert job.attempts == attempts_before
163+
assert job.claim_token is None
164+
assert job.lease_expires_at is None
165+
assert job.error == ""
166+
lead.draft.refresh_from_db()
167+
assert lead.draft.extracted_guesses == {}
168+
new_claim = claim_next_extraction()
169+
assert new_claim is not None
170+
assert new_claim.attempts == attempts_before + 1
171+
assert new_claim.claim_token != claimed.claim_token
126172

127173

128174
@override_settings(DOCUMENT_EXTRACTION_MAX_ATTEMPTS=1)

‎efile_app/efile/tests/test_security_boundaries.py‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
"""Regression checks for public routes, configuration input, and filing logs."""
22

33
import logging
4+
from copy import deepcopy
45
from unittest.mock import Mock, patch
56

67
import pytest
78
from django.contrib.sessions.backends.signed_cookies import SessionStore
89
from django.test import RequestFactory, override_settings
910

1011
from efile.api.filing_views import get_tyler_token
12+
from efile.utils.config_loader import config_loader
1113
from efile.views.session_api import forward_final_filing
1214

1315

@@ -28,6 +30,45 @@ def test_configuration_endpoints_reject_path_aliases(client, endpoint):
2830
assert response.status_code == 400
2931

3032

33+
@pytest.mark.django_db
34+
def test_case_type_configuration_without_jurisdiction_uses_cached_base(client):
35+
with patch.object(config_loader, "load_jurisdiction_config") as load:
36+
response = client.get("/api/case-type-config/")
37+
assert response.status_code == 200
38+
config = response.json()["config"]
39+
assert config["jurisdiction"] is None
40+
assert config["case_types"] == config_loader.base_config["base_case_types"]
41+
assert config["base_case_types"] == config_loader.base_config["base_case_types"]
42+
load.assert_not_called()
43+
44+
45+
@pytest.mark.django_db
46+
@pytest.mark.parametrize("jurisdiction", ["bogus", "../states/illinois"])
47+
def test_explicit_unknown_case_type_jurisdiction_still_rejected(client, jurisdiction):
48+
assert client.get("/api/case-type-config/", {"jurisdiction": jurisdiction}).status_code == 400
49+
50+
51+
@pytest.mark.django_db
52+
def test_template_with_unknown_query_jurisdiction_renders(client):
53+
response = client.get("/about/", {"jurisdiction": "bogus"})
54+
assert response.status_code == 200
55+
assert response.context["config"] == {}
56+
57+
58+
@pytest.mark.django_db
59+
@pytest.mark.parametrize("url", ["/missing-page/?jurisdiction=bogus", "/jurisdiction/bogus/missing-page/"])
60+
def test_unknown_jurisdiction_does_not_break_request_aware_404(client, settings, url):
61+
templates = deepcopy(settings.TEMPLATES)
62+
templates[0]["APP_DIRS"] = False
63+
templates[0]["OPTIONS"]["loaders"] = [
64+
("django.template.loaders.locmem.Loader", {"404.html": "missing-page-marker {{ config|length }}"})
65+
]
66+
with override_settings(DEBUG=False, TEMPLATES=templates):
67+
response = client.get(url)
68+
assert response.status_code == 404
69+
assert response.content == b"missing-page-marker 0"
70+
71+
3172
@pytest.mark.parametrize("status", [201, 400, 500])
3273
@override_settings(SUFFOLK_EFILE_API_KEY="secret-api-key-marker")
3374
def test_submission_logs_exclude_secrets_and_contents(caplog, status):

0 commit comments

Comments
 (0)