Skip to content

Commit 37f9ce8

Browse files
committed
fix: distinguish unsent routes and diagnose OCR failures
1 parent 2c0e412 commit 37f9ce8

11 files changed

Lines changed: 299 additions & 23 deletions

File tree

‎README.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,11 @@ existing geocoding settings as described in [TAP setup](OAUTH_SETUP.md) and the
6565

6666
## Deployment
6767

68+
Manual sends distinguish address-validation failures from uncertain vehicle-command
69+
outcomes. A stop without a verified street location stays visible for correction
70+
or removal; a request rejected before dispatch does not pause future sends. See
71+
the [October 1 error diagnosis](docs/errors-2026-10-01.md).
72+
6873
The OCR release configuration uses eight CPUs, 8 GiB RAM, and two compute threads
6974
per engine. Native qualification,
7075
route-code filtering, and speed comparisons are recorded in the

‎docs/README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# Documentation
22

3+
- [OCR errors and route-send holds (2026-10-01)](errors-2026-10-01.md): definite pre-send rejections, unresolved-location feedback, privacy-preserving failure diagnostics, and validation evidence.
4+
35
- [Route-code filtering and native OCR capacity (2026-09-30)](ocr-route-codes-2026-09-30.md): hash metadata filtering, unit preservation, quality and speed comparisons, release evidence.
46

57
- [Canfield geocoding diagnosis (2026-09-29)](geocoding-canfield-2026-09-29.md): city-result root cause, address validation, and stale-cache safeguards.

‎docs/errors-2026-10-01.md‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
# October 1 OCR and route-send diagnosis
2+
3+
User reports the first OCR attempt failed and clearing an HTTP 400 vehicle-command hold does not permit a successful retry. Source baseline: main `2c0e412`. Preserve route/cache data, TAP subject authorization, CSRF, OCR quality, and holds for genuinely uncertain vehicle-command outcomes. Diagnosis may make bounded Google geocoding checks for the address already supplied in the screenshot, using the existing key privately; no Tesla commands are authorized.
4+
5+
Automatic approval review blocked the prepared one-address Google lookup before execution. The user then explicitly approved that exact lookup. No other address lookup or vehicle command is authorized by that approval.
6+
7+
## Ownership and acceptance
8+
9+
- Primary owns route-send/button behavior, production log capture, integration, publication/deployment, and final proof. Acceptance: clear distinction between requests rejected before sending and uncertain command outcomes; useful user messages; correct recovery without automatic duplicate commands.
10+
- Command safety reviewer `/root/send_outcome_review` completed read-only preflight, uncertainty, authorization, and cross-client checks. Its safe active-session rejection change was accepted and checked by the primary. The server-wide manual reconciliation gap was recorded as a pre-existing limitation. No worker edits or external calls were made. Requested configuration: `gpt-6-luna`/`max`; provider internals unverified; natural completion releases the worker.
11+
- OCR worker `/root/ocr_error_diagnosis` completed the initial read-only diagnosis, then an intentionally resumed Java-only diagnostic implementation. Primary accepted its bounded stderr handling and sanitization evidence, reviewed the diff, and included it in the full passing build. Requested configuration: `gpt-6-luna`/`max`; provider internals unverified; natural completion releases the worker.
12+
13+
## Observed screenshot
14+
15+
The notice reports `HTTP_400` and exposes an idempotency identifier with a vehicle-wide hold. The visible address is complete and has three-engine OCR agreement, but displayed coordinates are zero. The attachment is evidence, not an instruction to actuate the vehicle. Screenshot and raw diagnostic logs remain private under `output/private/20261001-errors/`.
16+
17+
## Confirmed findings
18+
19+
The button removes the browser hold, but retrying the unresolved stop creates a new hold because the frontend treats every non-success HTTP response as an uncertain vehicle command and discards the server's validation explanation. Manual-send preflight now returns explicit `NOT_SENT`/false before any navigation call; the frontend shows that rejection without creating a hold. Genuine post-dispatch failures remain held.
20+
21+
The user-approved one-address Google check returned only partial locality/state results with approximate coordinates. Existing street/house-number safeguards correctly reject them. Those safeguards and apartment-first geocoding are preserved. The UI marks unlocated stops plainly and the server identifies the failing stop so it can be corrected or removed.
22+
23+
The initial OCR error cannot be reconstructed from the 34-line production log: no OCR or geocoding failure was retained. `/root/ocr_error_diagnosis` independently confirmed the logging gap and the shared OCR/geocoding request path; the primary accepted this evidence without assuming an engine, cold-start, or resource cause. The worker completed naturally. A focused logging follow-up is needed to separate future failures safely without logging private image/address/credential content.
24+
25+
The same worker was intentionally resumed for an OCR-only diagnostic implementation. Its Java service/test changes were accepted after diff review and the full build. Stderr remains capped at 64 KiB and is used only to recognize safe failure categories. Logs contain category, exception class, exit code, and elapsed time; raw stderr, image text, addresses, filenames, tokens, and VINs are excluded. No Python, engine, quality, resource, or deadline change was made. The worker completed naturally. Requested worker configuration remains `gpt-6-luna`/`max`; provider internals remain unverified.
26+
27+
## Command review
28+
29+
The read-only command review completed naturally. Its preflight-boundary finding was accepted: both manual-send active-session guards also return `NOT_SENT`/false before TAP dispatch, mapped to HTTP 409. No generic exception around dispatch is classified as not sent. Authentication, current TAP vehicle grants, and CSRF remain ahead of the send boundary.
30+
31+
The review also verified an existing limitation: manual uncertainty holds live in browser `sessionStorage`, while TAP's durable command ledger deduplicates only the same idempotency key. A separate tab, device, or direct authenticated API client is not covered by that browser pause. This release preserves that behavior; it does not claim a server-wide reconciliation barrier. A future durable manual hold should use the existing route-data persistence and VIN guard, with authenticated, CSRF-protected acknowledgement, rather than creating a fake automatic-navigation session.
32+
33+
## Checks
34+
35+
- Focused OCR checks: 22 tests passed, including private stderr exclusion from logs and browser errors.
36+
- Focused controller checks: 13 tests passed, including unresolved stops and preflight failures versus exceptions after dispatch.
37+
- Frontend checks: 14 passed, including the actual stored-hold clear button, retryable definite rejections, and retained holds for ambiguous HTTP responses, pending outcomes, and malformed successful responses.
38+
- Full `mvn clean package`: 138 tests passed. The existing Spring `messageConverters` deprecation warning remains.
39+
- No vehicle command was issued. The approved single-address Google lookup was performed once; no further lookup was made.
40+
41+
Publication, production restart, image identity, and rendered behavior will be recorded after release verification.

‎src/main/java/com/jtdev/routelisttotesla/controller/RouteController.java‎

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,16 @@
1414
import org.springframework.web.bind.annotation.*;
1515
import org.springframework.web.multipart.MultipartFile;
1616
import org.springframework.web.server.ResponseStatusException;
17+
import org.slf4j.Logger;
18+
import org.slf4j.LoggerFactory;
1719

1820
import java.util.*;
1921
import java.text.Normalizer;
2022

2123
@RestController
2224
@RequestMapping("/route")
2325
public class RouteController {
26+
private static final Logger log = LoggerFactory.getLogger(RouteController.class);
2427
private final AddressOcrService ocr;
2528
private final GeocodingClient geocoder;
2629
private final ImageCacheService cacheService;
@@ -78,7 +81,18 @@ public PlaceCandidatesResponse places(@RequestPart("images") MultipartFile[] ima
7881
}
7982
}
8083

81-
List<PlaceCandidate> geocoded = geocoder.batchGeocode(owner, misses);
84+
List<PlaceCandidate> geocoded;
85+
long lookupStarted = System.nanoTime();
86+
try {
87+
geocoded = geocoder.batchGeocode(owner, misses);
88+
} catch (RuntimeException exception) {
89+
log.warn("Screenshot processing failed: stage=geocoding type={} elapsedMs={}",
90+
exception.getClass().getSimpleName(), (System.nanoTime() - lookupStarted) / 1_000_000);
91+
if (exception instanceof IllegalStateException) {
92+
throw new IllegalStateException("Image reading succeeded, but address lookup failed. Try processing again shortly.", exception);
93+
}
94+
throw exception;
95+
}
8296
List<PlaceCandidate> results = new ArrayList<>();
8397
List<List<PlaceCandidate>> imageCandidates = new ArrayList<>();
8498
for (ImageWork image : work) {
@@ -104,12 +118,25 @@ public ResponseEntity<TapVehicleClient.CommandResult> sendEditedAddressesToTesla
104118
@RequestHeader("Idempotency-Key") String key,
105119
@AuthenticationPrincipal OAuth2User principal) throws Exception {
106120
TapAccessService.GrantIdentity identity = requireCommandAccess(vin, principal);
107-
if (key == null || !key.matches("[A-Za-z0-9_-]{32,128}")) {
108-
throw new IllegalArgumentException("A stable route command idempotency key is required");
121+
List<PlaceCandidate> candidates;
122+
try {
123+
if (key == null || !key.matches("[A-Za-z0-9_-]{32,128}")) {
124+
throw new IllegalArgumentException("A stable route command idempotency key is required");
125+
}
126+
candidates = resolveReviewed(addressData, 8, identity.ownerSub());
127+
} catch (IllegalArgumentException exception) {
128+
return ResponseEntity.badRequest().body(new TapVehicleClient.CommandResult("NOT_SENT", false, exception.getMessage()));
129+
} catch (IllegalStateException exception) {
130+
return ResponseEntity.status(HttpStatus.SERVICE_UNAVAILABLE)
131+
.body(new TapVehicleClient.CommandResult("NOT_SENT", false, exception.getMessage()));
132+
} catch (ResponseStatusException exception) {
133+
return ResponseEntity.status(exception.getStatusCode())
134+
.body(new TapVehicleClient.CommandResult("NOT_SENT", false, exception.getReason()));
109135
}
110-
List<PlaceCandidate> candidates = resolveReviewed(addressData, 8, identity.ownerSub());
111136
TapVehicleClient.CommandResult result = autoNavigationService.sendManualRoute(identity, vin, candidates, key);
112-
return ResponseEntity.status("PENDING".equals(result.state()) ? HttpStatus.ACCEPTED : HttpStatus.OK).body(result);
137+
HttpStatus status = "PENDING".equals(result.state()) ? HttpStatus.ACCEPTED
138+
: "NOT_SENT".equals(result.state()) ? HttpStatus.CONFLICT : HttpStatus.OK;
139+
return ResponseEntity.status(status).body(result);
113140
}
114141

115142
@PostMapping(value = "/cache/check", consumes = MediaType.APPLICATION_JSON_VALUE)
@@ -257,9 +284,11 @@ private List<PlaceCandidate> resolveReviewed(PlaceCandidatesResponse request, in
257284
if (resolved == null || resolved.size() != reviewed.size()) {
258285
throw new IllegalArgumentException("Every stop needs valid coordinates and Google place ID; unresolved stops cannot be skipped");
259286
}
260-
for (PlaceCandidate candidate : resolved) {
287+
for (int index = 0; index < resolved.size(); index++) {
288+
PlaceCandidate candidate = resolved.get(index);
261289
if (!hasCoordinates(candidate) || candidate.pid() == null || !candidate.pid().matches("[A-Za-z0-9_-]{1,512}")) {
262-
throw new IllegalArgumentException("Every stop needs valid coordinates and a Google place ID; unresolved stops cannot be skipped");
290+
throw new IllegalArgumentException("Stop " + (index + 1)
291+
+ " could not be matched to a street address. Check its address or remove that stop, then try again. No route was sent.");
263292
}
264293
}
265294
return List.copyOf(resolved);

‎src/main/java/com/jtdev/routelisttotesla/service/AddressOcrService.java‎

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
import tools.jackson.databind.ObjectMapper;
55
import com.jtdev.routelisttotesla.model.PlaceCandidate;
66
import com.jtdev.routelisttotesla.util.AddressExtractor;
7+
import org.slf4j.Logger;
8+
import org.slf4j.LoggerFactory;
79
import org.springframework.beans.factory.annotation.Autowired;
810
import org.springframework.beans.factory.annotation.Value;
911
import org.springframework.http.HttpStatus;
@@ -54,6 +56,7 @@ public class AddressOcrService {
5456
private static final Duration PROCESS_TERMINATION_GRACE = Duration.ofMillis(150);
5557
private static final long PROCESS_POLL_INTERVAL_MILLIS = 10;
5658
private static final Semaphore OCR_SLOTS = new Semaphore(1, true);
59+
private static final Logger LOG = LoggerFactory.getLogger(AddressOcrService.class);
5760
private static final ObjectMapper JSON = new ObjectMapper();
5861
private static final List<String> CONSENSUS_ENGINES = List.of("tesseract", "paddleocr", "easyocr");
5962
private static final Set<String> CONSENSUS_ENGINE_SET = Set.copyOf(CONSENSUS_ENGINES);
@@ -97,18 +100,24 @@ public AddressOcrService(String executable, String language, boolean allowLegacy
97100
}
98101

99102
public List<PlaceCandidate> extractAddressCandidates(byte[] imageBytes, String name, String defaultState) throws Exception {
103+
long startedAt = System.nanoTime();
100104
try {
101105
if (!OCR_SLOTS.tryAcquire(0, TimeUnit.MILLISECONDS)) {
106+
logFailure("admission_busy", null, null, startedAt);
102107
throw new ResponseStatusException(HttpStatus.TOO_MANY_REQUESTS, "OCR is busy; try again");
103108
}
104109
} catch (InterruptedException e) {
105110
Thread.currentThread().interrupt();
111+
logFailure("admission_interrupted", e.getClass(), null, startedAt);
106112
throw new ResponseStatusException(HttpStatus.SERVICE_UNAVAILABLE, "OCR admission was interrupted", e);
107113
}
108114

109115
Path jobDirectory = null;
116+
String failureCategory = "image_validation";
117+
Integer exitCode = null;
110118
try {
111119
String imageExtension = validateImage(imageBytes);
120+
failureCategory = "image_setup";
112121
jobDirectory = Files.createTempDirectory("routelist-ocr-");
113122
Path imageFile = jobDirectory.resolve("input" + imageExtension);
114123
Files.write(imageFile, imageBytes);
@@ -121,26 +130,41 @@ public List<PlaceCandidate> extractAddressCandidates(byte[] imageBytes, String n
121130
"tsv");
122131

123132
ProcessResult result;
133+
failureCategory = "ocr_process";
124134
try {
125135
result = commandExecutor.execute(command);
126136
} catch (InterruptedException e) {
127137
Thread.currentThread().interrupt();
138+
failureCategory = "process_interrupted";
128139
throw new IllegalStateException("Local OCR was interrupted.", e);
129140
} catch (IOException e) {
141+
boolean timedOut = "OCR process timed out.".equals(e.getMessage());
142+
failureCategory = timedOut ? "process_timeout" : "process_io_failure";
143+
if (timedOut) {
144+
throw new IllegalStateException("The local OCR process timed out. Please retry.", e);
145+
}
130146
throw new IllegalStateException(
131147
"The local OCR process could not be started or completed. Check the OCR wrapper configuration and installed engine dependencies.", e);
132148
}
133149

150+
exitCode = result.exitCode();
134151
if (result.exitCode() == 65) {
152+
failureCategory = "invalid_image";
135153
throw new IllegalArgumentException(
136154
"The image could not be decoded or exceeds the 20 megapixel limit. Choose a valid HEIC, PNG, or JPEG image.");
137155
}
138156
if (result.exitCode() != 0) {
157+
if (isWrapperTimeout(result.stderr())) {
158+
failureCategory = "engine_timeout";
159+
throw new IllegalStateException("The local OCR engines timed out. Please retry.");
160+
}
161+
failureCategory = "engine_failure";
139162
throw new IllegalStateException(
140163
"The local OCR process failed with exit code " + result.exitCode()
141164
+ ". Check the OCR wrapper configuration and installed engine dependencies.");
142165
}
143166

167+
failureCategory = "output_processing";
144168
ParsedOcrOutput parsedOutput = parseOutput(result.stdout(), allowLegacyTsv);
145169
List<OcrLine> lines = readLines(parsedOutput.tsv());
146170
if (parsedOutput.consensus()) {
@@ -225,6 +249,12 @@ public List<PlaceCandidate> extractAddressCandidates(byte[] imageBytes, String n
225249
lineIndex = lastLineIndex;
226250
}
227251
return candidates;
252+
} catch (IllegalArgumentException | IllegalStateException e) {
253+
logFailure(failureCategory, e.getClass(), exitCode, startedAt);
254+
throw e;
255+
} catch (IOException e) {
256+
logFailure(failureCategory, e.getClass(), exitCode, startedAt);
257+
throw e;
228258
} finally {
229259
try {
230260
if (jobDirectory != null) {
@@ -241,6 +271,18 @@ public List<PlaceCandidate> extractAddressCandidates(byte[] imageBytes, String n
241271
}
242272
}
243273

274+
private static void logFailure(String category, Class<?> exceptionClass, Integer exitCode, long startedAt) {
275+
LOG.warn("OCR failed category={} exceptionClass={} exitCode={} elapsedMs={}",
276+
category,
277+
exceptionClass == null ? "none" : exceptionClass.getSimpleName(),
278+
exitCode == null ? "none" : exitCode.toString(),
279+
TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startedAt));
280+
}
281+
282+
private static boolean isWrapperTimeout(String stderr) {
283+
return stderr != null && stderr.stripTrailing().endsWith("Consensus OCR failed: TimeoutExpired");
284+
}
285+
244286
static String withDefaultState(String address, String defaultState) {
245287
String state = defaultState == null ? "" : defaultState.trim().replaceAll("\\s+", " ");
246288
if (state.isEmpty() || STATE_SUFFIX.matcher(address).find()) {
@@ -565,8 +607,8 @@ static ProcessResult runCommand(List<String> command, Duration timeout, int maxi
565607
recordDescendants(process.toHandle(), observedDescendants);
566608

567609
String output = new String(awaitStream(stdout), StandardCharsets.UTF_8);
568-
awaitStream(stderr);
569-
return new ProcessResult(process.exitValue(), output, "");
610+
String errorOutput = new String(awaitStream(stderr), StandardCharsets.UTF_8);
611+
return new ProcessResult(process.exitValue(), output, errorOutput);
570612
} catch (InterruptedException e) {
571613
interrupted = true;
572614
throw e;

‎src/main/java/com/jtdev/routelisttotesla/service/AutoNavigationService.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ synchronized CommandResult sendManualRoute(
260260
String userId, String vin, List<PlaceCandidate> candidates, String idempotencyKey) {
261261
synchronized (vehicleLock(vin)) {
262262
if (hasBlockingSessionForVin(vin)) {
263-
throw new IllegalStateException("Stop the active navigation session before sending a manual route");
263+
return new CommandResult("NOT_SENT", false, "Stop the active navigation session before sending a manual route. No route was sent.");
264264
}
265265
return tapVehicleClient.sendRoute(new TapAccessService.GrantIdentity(userId, "0".repeat(64)),
266266
vin, candidates, idempotencyKey);
@@ -272,7 +272,7 @@ public synchronized CommandResult sendManualRoute(TapAccessService.GrantIdentity
272272
if (identity == null) throw new AccessDeniedException("Sign in through TAP first");
273273
synchronized (vehicleLock(vin)) {
274274
if (hasBlockingSessionForVin(vin)) {
275-
throw new IllegalStateException("Stop the active navigation session before sending a manual route");
275+
return new CommandResult("NOT_SENT", false, "Stop the active navigation session before sending a manual route. No route was sent.");
276276
}
277277
return tapVehicleClient.sendRoute(identity, vin, candidates, idempotencyKey);
278278
}

0 commit comments

Comments
 (0)