[suryapilla]

📋 Summary

Fixes misaligned validation results when upstream rows were dropped or reordered. ValidationColumnGenerator now builds its results with the input frame's index, and raises a clear error if a validator returns a different number of results than records.

🔗 Related Issue

Fixes #1073

🔄 Changes

• Build the validation results DataFrame with index=data.index, so results stay aligned to their rows. Previously a fresh 0..n-1 index plus pd.concat(..., axis=1) produced extra rows when the input index had gaps (returned 4 rows but 3 were expected), and could attach results to the wrong rows when the index was reordered (as @ting-hong-shieh found on the issue).
• Raise DataDesignerRuntimeError when a validator returns a different number of results than records. Previously this was silently padded with empty rows.
• Updated test_validation_column_generator_generate_with_different_strategies: its mock returned one result per batch instead of one per record, and only passed because of the silent padding. It now returns one result per record, like real validators.

🧪 Testing

• make test passes
• Unit tests added/updated
• E2E tests added/updated (if applicable)

New tests:

• test_validation_column_generator_preserves_non_contiguous_index: fails on main (4 rows for 3), passes with the fix
• test_validation_column_generator_preserves_reordered_index: fails on main (results on the wrong rows), passes with the fix
• test_validation_column_generator_raises_on_result_count_mismatch: covers the new error

make check-all-fix passes.

✅ Checklist

• Follows commit message conventions
• Commits are signed off (DCO)
• If I am an external contributor, the linked issue is open and labeled triaged
• Architecture docs updated (if applicable)

Prepared with AI assistance; I reviewed and tested the changes locally.

[github-actions]

All contributors have signed the DCO ✍️ ✅
_{Posted by the DCO Assistant Lite bot.}

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

Keeps validation results attached to the correct input rows.

• Validation results stay attached to their input rows.
• Each validator batch must return one result per record.

_{Reviews (3) · Last reviewed commit: "Merge branch 'main' into fix/validation-..." · Reviewed by Greptile}

[suryapilla]

I have read the DCO document and I hereby sign the DCO.

[nabinchha]

Thanks for the fix, @suryapilla!

Summary

Preserving the input index fixes validation-result alignment after upstream rows are dropped or reordered, matching the reported issue. The new total-count guard is useful, but count validation also needs to happen per batch to prevent incorrect assignments.

Findings

Warnings — Worth addressing

packages/data-designer-engine/src/data_designer/engine/column_generators/generators/validation.py:102 — Check each batch's result count before combining outputs

• What: The new guard checks only the combined count. For four inputs split into batches of two, responses containing one and three results still total four and pass the check. This behavior predates the PR; the request is to complete the count validation being added here.
• Why: Results can silently attach to the wrong records. In the reproduction, inputs with IDs [0, 1, 2, 3] receive results for IDs [0, 2, 3, 3] despite the correct total count.
• Suggestion: Validate each batch's result count against its input count before flattening, covering both sequential and parallel execution. Add a regression where short and excess responses cancel in the aggregate, and ensure malformed batches cannot shift another batch's results.

packages/data-designer-engine/tests/engine/column_generators/generators/test_validation.py:283 — Cover empty and excess validator results

• What: The mismatch test covers one returned result for three inputs, but does not cover empty or excess results.
• Why: A guard that rejects only short responses would still pass this test, leaving excess-result handling unprotected. The current production guard correctly handles these total-count cases.
• Suggestion: Parametrize the returned count over 0, 1, and 4 for three input records, and assert the corresponding error message.

What Looks Good

• Preserving data.index fixes extra rows after upstream drops and incorrect assignments after reordering.
• The regression tests verify per-row validation values; all three fail against the parent implementation and pass with this fix.
• The batching mock now returns one result per input record, matching the validator contract.

Residual Risk

All 740 tests across column generators, validators, and dataset builders passed, along with lint and formatting checks. Live remote services, the full repository suite, and the supported Python/pandas version matrix were not exercised.

Verdict

Needs changes — enforce per-batch result counts and add coverage for empty, excess, and compensating batch responses before merging.

---

This review was generated by an AI assistant.

[suryapilla]

Thanks for the review, @nabinchha Both points are addressed in e074cd8.

Moved the count check into _validate_batch(), so every batch is checked before results are flattened. Both the sequential and parallel paths go through it. I removed the total-count check, since per-batch validation makes it unreachable. Happy to restore it as a backstop if you'd prefer.

• Sequential: a mismatched batch raises DataDesignerRuntimeError.
• Parallel: a mismatched batch goes through the existing error callback and becomes ValidationResult.empty() for that batch only, so it can't shift results onto another batch. This is logged rather than failing the whole run, matching how the parallel path already handles validator errors. Happy to make it fail loudly there too if you'd prefer.

Tests:

• Sequential: a short batch and an excess batch that cancel out in the total now raise.
• Parallel: a short first batch gets None placeholders while the good batch keeps its own results ([None, None, False, False]).
• The mismatch test is now parametrized over 0, 1 and 4 results for 3 records.

[nabinchha]

Thanks for addressing the feedback, @suryapilla!

Summary

Reviewed e074cd89. The per-batch count check and expanded regression coverage address both previous requests, while preserving the input-index alignment fix.

Findings

Both previous findings are resolved. No new actionable issues found.

What Looks Good

• Sequential and parallel validation now check each batch before combining results, preventing short and excess responses from cancelling out. The previous total-count check is redundant with this validation in place.
• Parallel failures retain the existing batch-local None placeholders, preserving unaffected results and row identities. Keeping the existing error policy is appropriate here.
• Tests cover empty, short, excess, and compensating responses, plus parallel placeholder placement and noncontiguous/reordered indexes.

Residual Risk

All 744 tests across column generators, validators, and dataset builders passed, along with lint and formatting checks. Live remote services, the full repository suite, and the supported Python/pandas version matrix were not exercised.

Verdict

Ship it — both requested changes are addressed; no remaining findings.

---

This review was generated by an AI assistant.