[lipikaramaswamy]

Related Issue

No linked issue required: maintainer-requested shared DataDesigner upgrade and JSON-storage prerequisite for the nested-column handoff reported on #305.

Related: #331 remains unresolved by this change; this PR does not close it.

Plan Document

plans/dd-json-storage/plan.md

Summary

• Raise the DataDesigner minimum to data-designer>=0.10.0,<0.11 (lockfile resolves DD/config/engine 0.10.0), and update the notebook Pillow range to compatible 13.x.
• Preserve DataDesigner's JSON-storage metadata at the shared NDD adapter boundary for an explicit registry of nested engine columns, for both workflow execution and exported configs.
• Normalize registered and explicitly planned nested seed columns to plain Python values, including Arrow-backed columns, object-dtype NumPy arrays/scalars, and top-level missing values. Preserve existing field-level storage contracts and declare JSON storage for registered named outputs. Do not infer JSON from strings or alter unrelated user columns.
• Cover real Parquet write/read/write and mixed empty/populated row groups (buffer_size=2) carried into both production Substitute-map generation and latent detection. Assert preserved record order, IDs, and diagnostic entity spans.
• Wrap storage metadata validation and seed Parquet encoding failures in AnonymizerWorkflowError with the original exception chained, for run, preview, and exported configs.

Scope and follow-up for #305

This is a shared prerequisite, not a claim that Aaron's complete #305 report is fixed. Main does not contain COL_VALIDATION_INVENTORIES; #305 must register that constant after rebasing. DD 0.10 cannot independently declare JSON storage for plugin side-effect outputs, so initial inventory generation may also require a named output generator or upstream side-effect storage support.

A deterministic all-failed DataDesigner generation test still raises AnonymizerWorkflowError caused by DataDesignerGenerationError. The missing-output/all-failed behavior in #331 is separate and deliberately unchanged.

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update
• Refactoring
• CI, release, or contributor workflow update

Contributor Checklist

• PR title follows Conventional Commits
• Maintainer-owned no-issue reason documented above
• Plan document linked above
• Public API impact checked; no public signatures or bundled-skill changes needed
• No real PII added; regression fixtures are synthetic
• No credentials or real endpoint secrets added

Validation

• make check: formatting, Ruff, full typecheck, lock freshness, and SPDX checks passed. Commit hooks and DCO signoff passed.
• pytest tests/engine/test_ndd_adapter.py tests/engine/test_ndd_storage.py -q: 37 passed.
• Six regression cases for malformed storage metadata and invalid JSON encoding failed before the error-boundary fix and pass after, across run, preview, and export.
• Both production handoff regressions now use four rows and buffer_size=2: an empty-entity row group followed by a populated group. Model generation and readiness are stubbed; scheduling, prompt rendering, parsing, and Parquet I/O are real.
• Full make test with a short temporary path and local socket access: 1,776 passed, 2 skipped.
• Fresh, non-editable wheel installed over the hash-locked runtime/dev dependencies: 37 adapter/storage tests passed from outside the checkout in Python isolated mode, with imports verified from site-packages. Dependency compatibility passed.
• The lockfile still resolves DataDesigner/config/engine 0.10.0; narrowing the upper bound to <0.11 changed no resolved package versions.
• Rebased onto current main, preserving fix(replace): fail records with malformed entity payloads #367's strict entity parsing alongside fix(ndd): encode registered seed columns from plain Python values #378's shared converter. The all-failed test explicitly records the separate fix(rewrite): handle all-failed repair batches without missing-Parquet crashes #331 follow-up.
• No live model inference or campaign reruns were performed. Existing dependency deprecation warnings remain.

Documentation and Artifacts

• Implementation plan documents behavior, validation, and limitations; no public API documentation changes needed
• If docs changed: make docs-build passes locally (not run; no site documentation changes)
• If tutorial sources changed: notebooks regenerated (not applicable)
• Relevant local workflow/storage validation listed above; no provider/prompt/detection-quality changes

[greptile-apps]

Confidence Score: 5/5

[High impact] The PR appears safe to merge; no new blocking issues were found.

Summary

The PR preserves nested engine columns across DataDesigner workflow handoffs.

• Nested values keep their shape across NDD workflow handoffs.
• NDD now requires the DataDesigner release that supports JSON storage.

No new actionable issues were found in the changes since the previous review.

_{Reviews (4) · Last reviewed commit: "fix(ndd): wrap seed errors and verify mu..." · Reviewed by Greptile}

[memadi-nv]

A Non Blocking Consideration:

Anonymizer already handles nested NumPy arrays from Parquet reloads. Could we add a test confirming those inputs still work with the new JSON storage? Codex reproduced a failure with an ordinary pandas reload, though the normal DD reader path passes.

[binaryaaron]

agent (review-pr): The blocking issue is Andre's thread at storage.py:84; #378 fixes it against this branch. The points below are separate from that fix.

1. Should-fix: the tests do not cover more than one batch, and the description says they do. Every DataDesigner test in tests/engine/test_ndd_storage.py uses a single record and never sets buffer_size, but the description claims "two-batch DataDesigner coverage". That gap is why the multi-batch failure was not caught. #378 adds one adapter-level test with buffer_size=2; a Substitute or latent-detection handoff test with two row groups would cover the production path. Please also correct the description.
2. Should-fix: the error escapes as DataDesigner's InvalidFileFormatError, not AnonymizerWorkflowError. prepare_json_storage_seed runs outside the adapter's try (adapter.py:352 and :515), so any encode failure on the seed reaches the caller as a raw DataDesigner exception. Moving the call inside the try, or wrapping it, keeps the adapter's error contract.
3. Should-fix: pyproject.toml:11 widens the cap to data-designer>=0.10.0,<2. DataDesigner is pre-1.0, and 0.9 to 0.10 changed storage behavior that this PR depends on. Main capped at <0.10; <0.11 keeps that discipline.
4. Nit: test_dd010_all_failed_batch_still_raises_not_failed_records pins the #331 behavior. It will need flipping when #331 lands; a comment naming #331 would make that clear.
5. Nit: plans/dd-json-storage/plan.md records run-specific counts ("1,613 passed", "113 hash-locked…"), which will go stale.

On @memadi-nv's question about nested NumPy arrays from Parquet reloads: #378 converts them before encoding, and test_seed_values_are_plain_python_before_json_encoding[numpy] covers that case.

[lipikaramaswamy]

@memadi-nv — your NumPy reload concern is addressed by the merged #378 and the follow-up in f858c4b. test_seed_values_are_plain_python_before_json_encoding[numpy] covers an input containing nested NumPy arrays and scalars, including the shape produced by Parquet reloads. The production handoff tests now additionally exercise mixed empty/populated row groups through DataDesigner's own batch reader into both Substitute-map generation and latent detection. All 37 adapter/storage tests pass in the checkout and against a fresh installed wheel.

[lipikaramaswamy]

@binaryaaron — addressed all five points from your review in f858c4bb:

1. Multi-batch production coverage: both Substitute-map and latent-detection handoff tests now use four rows with buffer_size=2, with empty entities in the first row group and populated entities in the second. They check retained diagnostic spans, record IDs, order, and failed records. The PR description now describes this coverage precisely.
2. Canonical errors: seed metadata validation and Parquet encoding are inside the workflow error boundary for run/preview, and wrapped for config export. Six regression cases check AnonymizerWorkflowError with the original InvalidFileFormatError chained; all six failed before this fix and pass after it.
3. Dependency cap: narrowed to data-designer>=0.10.0,<0.11 in pyproject.toml and the lockfile. Resolved package versions remain unchanged at DD/config/engine 0.10.0.
4. fix(rewrite): handle all-failed repair batches without missing-Parquet crashes #331: added an explicit comment to the all-failed test identifying that separate follow-up; its current exception behavior is unchanged.
5. Plan cleanup: removed revision-specific test/dependency counts from the plan and kept current validation results in the PR description.

#378 is merged into this branch, and the rebase onto main preserves #367's strict parser alongside the shared converter. Validation: make check and commit hooks passed; full suite 1,776 passed, 2 skipped; 37 adapter/storage tests passed against a fresh non-editable wheel, with imports verified from site-packages and dependency compatibility checked.

[binaryaaron]

agent (review-pr): Rechecked at f858c4b. The points from my earlier review are addressed:

• Both handoff tests now run four rows with buffer_size=2 (an empty row group, then a populated one) and check record order, IDs and entity spans.
• Seed preparation and the seed write raise AnonymizerWorkflowError with the cause chained on run, preview and build_config, and the new six-case test covers all three. DataDesigner's encode errors name only the column and row, so no record values reach the WARNING log.
• The cap is <0.11, the fix(rewrite): handle all-failed repair batches without missing-Parquet crashes #331 comment is on the all-failed test, and the plan no longer carries run counts.

Two notes, neither blocking:

• A seed failure now goes through the generic handler, so the warning reads "Workflow failed for N input record(s) on model(s) ..." although no model ran.
• to_python encodes every value the current schemas put in a planned column. One case still fails to encode: a numeric list with a null in a registered side-effect column that is split across row groups. DataDesigner returns it as array([1., nan]), and the next seed write raises. No current schema has such a field. If one is added, converting nested NaN and pd.NA to None in storage.py would cover it; restoring the ints needs DataDesigner to unify fragment schemas on read.

[lipikaramaswamy]

@binaryaaron — thanks for the recheck. Agreed that these two notes are nonblocking for the current schemas.

• The warning lists configured model aliases; it does not distinguish a seed-preparation failure from a generation failure. That wording can be clarified separately without changing the error contract.
• I checked the registered columns and current schema definitions: the nullable numeric-list case is outside the shapes produced today. Keeping recursive nested-null normalization out of this PR avoids changing missing-value semantics without a concrete schema requirement. If such a field is introduced, it needs a multi-row-group regression covering nested NaN/pd.NA conversion; restoring integer types remains a DataDesigner fragment-schema concern.

No additional code changes in response to these notes. The earlier fixes and validation remain at f858c4b.

[binaryaaron]

agent (review-pr): @lipikaramaswamy: a case for the nested-null question on #375. #333 adds _sensitivity_disposition, whose low_sensitivity_reason is null in every entry whenever a batch has no low entities (always true in strict mode). On the merge of #333 with main, that column is stored as an Arrow struct with a null-typed child, and df.to_dict("records") in _run_evaluate_repair_loop raises ArrowIndexError. It reproduced live on 5 rows. The fix that held registers the two disposition columns for JSON storage in engine/ndd/storage.py, as #375 does for other engine-owned nested columns. #333 is the first nullable nested field outside JSON storage, so the check on #375 found no reachable bug until now. A tested patch is on offer for #333.