fix(accounting): dropped records fail the run; JSON read-layer fails fast (#234, #235)

[LukasWodka]

Summary

PR2 of the coercion-cluster fix. Closes #234 and resolves the consistency requirement of #235.

Both issues are facets of the same silent-data-loss class #99/#223/#230 have been closing: a record gets dropped, but the run still exits 0 and the K8s Job is marked Succeeded.

A surprising finding that reshaped the fix

The empirical end-to-end behaviour contradicts #235's one-line framing. Through the real ingest() path, both CSV and JSON already fail-fast at the DataValidator gate on a bad cell:

CSV  one-bad-cell: RAISED ValueError: Data Validator Validator failed
JSON one-bad-cell: RAISED ValueError: Data Validator Validator failed   ← not a silent skip

The "JSON silently skips" behaviour only occurs at the read_data level (bypassing the gate) or for records the gate misses. So the genuine, dominant bug is #234's silent skips, and #235's consistency requirement is met by making the read-layer fail-fast like the gate already does.

#234 — close the silent-skip channels

• A record dropped by process_record (blank/invalid unique_id, or a processing error) is now appended to failed_records, so run_ingestion exits non-zero instead of returning [] and reporting clean success.
• IngestionSummary.has_failures and the summary's total_failures now include skipped_records — fixing the self-contradiction where a run printed "0 failure(s)" while the success-rate text said "most records failed".
• An invalid intent now fails fast (raises before any lock/DB work). It's a run-wide config error, not a per-row skip; previously it silently dropped every record and still exited 0.

#235 — make CSV and JSON consistent

• JSONIngestor._iter_validated_records now raises on a type-invalid / non-object record instead of silently skipping it — consistent with the CSV cast and the DataValidator gate. A bad record aborts with a clear non-zero exit instead of vanishing with only a log line nobody durably sees.

Verification

• tests/test_ingestor_base.py: split the old "skips on bad intent" test into test_ingest_fails_fast_on_invalid_intent + test_ingest_counts_dropped_record_as_failure, and added test_ingest_keeps_good_records_and_counts_dropped (the canonical mixed-run scenario: good rows ingest, dropped rows force non-zero exit, summary trips has_failures). Added a skipped_records case to the has_failures matrix.
• tests/test_json_ingestor.py: the two "skips bad record" tests rewritten to assert the fail-fast raise.
• Full suite: 965 passed, 1 xfailed (pre-existing semseg feat: wire segmentation mask sidecars through the declarative path #136), coverage 97.2% (gate 95%).

Deferred (intentional)

Per-row tolerance — keep the good rows, drop+count the bad ones, the "preferred direction" noted in #235 — is not in this PR. It requires the DataValidator gate to stop blocking on per-cell type errors (it currently aborts, and ~40 validator tests assert that). That's a change to the gate's role, best done as part of the validator/registry refactor rather than bundled into this correctness fix. This PR delivers the hard requirement (consistent behaviour + no partial run reported as success); the per-row UX improvement is a clean follow-up on top.

Relationship to PR #242

Independent of #242 (the coercion source-of-truth). The two touch different functions (base.py accounting + json_ingestor read-loop here; coercion module + casts there) and merge cleanly.

🤖 Generated with Claude Code

[LukasWodka]

👋 Heads-up — Code review queue is at 11 / 8

Above the WIP limit. The team convention is to review existing PRs before opening new work.

Open PRs currently in Code review (oldest first):

• client#254 — Sync develop → main for v1.7.1 chart release (egress-enforcement preflight helm test, inert) · author: @saadqbal · no reviewer assigned
• client#255 — fix(egress-proxy): enforcement check probes TCP reachability, not HTTP code [image_validator falsely rejects images when target_size matches exactly #104] · author: @saadqbal · no reviewer assigned
• data-ingestors#233 — fix(validators): accept single-dict JSON top-level shape (issue DataValidator rejects single-dict JSON inputs that JSONIngestor.read_data is designed to handle #232) · author: @divyasinghds · no reviewer assigned
• data-ingestors#240 — fix: remove instance_segmentation zombie category, add dispatch-site congruence guard · author: @LukasWodka · no reviewer assigned
• data-ingestors#241 — fix: report real MySQL column types from get_table_schema (dialect-type reflection) · author: @LukasWodka · no reviewer assigned
• data-ingestors#242 — fix(coercion): single source of truth for NA policy + int64 range (bug: huge integer in INT column raises cryptic numpy "ufunc 'isinf' not supported" instead of a clean overflow error #236, bug: NA/null/None tokens crash non-tabular CSV ingest but are NULL for tabular — validator passes them (validate-pass → ingest-crash) #237) · author: @LukasWodka · no reviewer assigned
• frontend-app#521 — Prod · author: @waqaskhanroghani · reviewer: @saadqbal
• frontend-app#522 — STG · author: @waqaskhanroghani · no reviewer assigned
• tracebloc-py-package#217 — fix(sdk): accept NumPy integer counts; framework-aware custom-loss header · author: @divyasinghds · no reviewer assigned
• tracebloc-py-package#218 — fix(upload): output-shape validation + hard-fail class-count check for text_classification · author: @shujaatTracebloc · reviewer: @aptracebloc

Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.)