fix: add visibility to silent failures in NDD adapter#136
Conversation
…circuits in adapter logs - Wrap `preview` and `create`+`load_dataset` in try/except that logs a user-facing WARNING (row count, model aliases, `type(exc).__name__`, message, actionable hint) plus a DEBUG breadcrumb (`workflow_name`, column list), then re-raises unchanged. - Warn in `_detect_missing_records` on both short-circuits with distinct framing: input-missing as a correctness-facing silent-drop / invariant violation, output-missing as an observability improvement with a column-diff breadcrumb. - Tests cover each new WARNING/DEBUG path and assert the new log messages contain no backend references. Made-with: Cursor
Greptile SummaryThis PR surfaces previously silent failures in Confidence Score: 5/5Safe to merge — all logic is correct, tests cover all new paths, and the only finding is a speculative P2 concern about str(exc) embedding backend names. No P0 or P1 issues found. The sole inline comment is P2 (potential backend name leakage through str(exc)), which is speculative and doesn't affect correctness. Tests are well-structured and assertions align precisely with the actual log format strings. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[run_workflow called] --> B[_attach_record_ids]
B --> C[build config + compute record_count]
C --> D{preview_num_records?}
D -- None --> E[data_designer.create + load_dataset]
D -- set --> F[data_designer.preview]
E --> G{Exception?}
F --> G
G -- yes --> H[log WARNING + DEBUG]
H --> I[raise AnonymizerWorkflowError from exc]
G -- no --> J[_detect_missing_records]
J --> K{RECORD_ID_COLUMN in input_df?}
K -- no --> L[log WARNING + DEBUG\nreturn empty list]
K -- yes --> M{RECORD_ID_COLUMN in output_df?}
M -- no --> N[log WARNING + DEBUG\nreturn all as FailedRecord]
M -- yes --> O[diff input_ids vs output_ids\nreturn FailedRecord list]
O --> P[WorkflowRunResult]
N --> P
L --> P
Reviews (5): Last reviewed commit: "update test" | Re-trigger Greptile |
Signed-off-by: memadi <memadi@nvidia.com>
Signed-off-by: memadi <memadi@nvidia.com>
lipikaramaswamy
left a comment
There was a problem hiding this comment.
Looks good to approve. Were you able to check using a small model or config with very low timeout to simulate a data designer failure?
Signed-off-by: memadi <memadi@nvidia.com>
Instead of driving an actual low-timeout model, the tests added in
Each test asserts that:
I assume that's sufficient, but let me know if it's not the case. |
Signed-off-by: memadi <memadi@nvidia.com>
Summary
Surface previously silent failure modes in
NddAdaptervia structured logs, and introduce a typed error wrapper so callers don't depend on backend exception types.Changes
src/anonymizer/engine/ndd/adapter.pycreate()/load_dataset()/preview()in atry/exceptthat logs aWARNING(row count, model aliases, exception) +DEBUG(workflow name, columns) and re-raises asAnonymizerWorkflowErrorwith__cause__preserved.WARNING+DEBUGlogs for the two silent short-circuits in_detect_missing_records(input missing tracking column, output missing tracking column, with a column-diff breadcrumb).src/anonymizer/interface/errors.pyAnonymizerWorkflowErrorfor wrapping workflow-step failures.tests/engine/test_ndd_adapter.pyType of Change
Testing
make testpasses locallymake checkpasses locallyRelated Issues
Plan for this issue created in draft PR #128 => some parts partially changed
Closes #110