Skip to content

feat: expose terminal failure locations - #861

Draft
andreatnvidia wants to merge 1 commit into
mainfrom
andreatnvidia/feat/terminal-failure-locator
Draft

feat: expose terminal failure locations#861
andreatnvidia wants to merge 1 commit into
mainfrom
andreatnvidia/feat/terminal-failure-locator

Conversation

@andreatnvidia

Copy link
Copy Markdown
Contributor

📋 Summary

Add an opt-in, lightweight terminal-failure locator for callers that need to reconcile omitted rows without enabling full task tracing. This supports merged Anonymizer pipelines while leaving the default generation path unchanged.

🔗 Related Issue

Closes #860

🔄 Changes

  • Add capture_terminal_failures to create(), acreate(), and preview().
  • Return typed (seed_row_index, column) terminal failures on successful results and zero-row generation errors.
  • Record only failures that remain terminal after retry and salvage, excluding skipped, recovered, downstream-unrun, and early-shutdown-cancelled tasks.
  • Expand batch failures to affected global row positions and scope resume results to the current invocation.
  • Expose early_shutdown on successful results so callers can detect when cancelled rows have no truthful column attribution.
  • Keep collection disabled by default and retain only O(number of terminal failures) records when enabled.

🔍 Attention Areas

⚠️ Reviewers: Please pay special attention to the public API contract and early-shutdown behavior.

  • seed_row_index is the requested generation-sequence position. It maps directly to the raw seed row only for compatible ordered seeding.
  • terminal_failures excludes rows cancelled by global early shutdown. Callers should check early_shutdown before treating the list as complete.

🧪 Testing

  • Config suite: 644 passed
  • Engine suite: 2,262 passed
  • Interface suite: 1,121 passed, 1 skipped
  • Unit tests added and updated
  • E2E tests: N/A - no external provider behavior changed

✅ Checklist

  • Follows commit message conventions
  • Commits are signed off (DCO)
  • Architecture docs updated (N/A - no architectural change)

Add opt-in terminal failure capture for create and preview results, including zero-row generation errors. Surface early shutdown so callers can distinguish cancelled rows from attributable terminal failures.

Closes #860

Signed-off-by: Andre Manoel <amanoel@nvidia.com>
@github-actions

Copy link
Copy Markdown
Contributor

Fern preview: https://nvidia-preview-pr-861.docs.buildwithfern.com/nemo/datadesigner

Fern previews include the docs-website version archive with PR changes synced into latest. Notebook tutorials are rendered without execution outputs in previews.

@nabinchha

nabinchha commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Thanks for putting this together, @andreatnvidia — the lightweight, opt-in shape is a good fit for callers that need omission provenance without retaining full task traces.

Summary

This adds typed terminal-failure locations to create, async create, preview, successful results, and zero-row errors, while preserving retry, skip, resume, and early-shutdown semantics. I reviewed the public contract, scheduler paths, interface plumbing, existing feedback, and documentation coverage, then ran the focused config/engine/interface tests plus Ruff checks.

Findings

Critical — Let's fix this before merge

packages/data-designer-engine/src/data_designer/engine/dataset_builders/async_scheduler.py:2355 — Expression-driven row omissions are never recorded

  • What: _record_terminal_failure() is called when a generator task raises and is terminally dropped, but expression generators normally represent per-row render/cast failures by returning a DataFrame without those row indexes. Both expression row-drop merge paths (_run_full_column_from_scratch_with_row_drops() here and _run_batch() around line 2510) call _drop_row() directly for a missing result and never append a TerminalTaskFailure. For example, the existing test_expression_row_drops_shrink_async_row_group case drops seed row 1; enabling capture_terminal_failures=True would still leave scheduler.terminal_failures == [].
  • Why: The issue and PR promise provenance for every terminally omitted seed row, explicitly including expression columns and other row-producing column types. A caller can therefore receive early_shutdown=False and an apparently complete but empty failure list even though rows were omitted, defeating the reconciliation contract this change introduces.
  • Suggestion: Record the originating task/row before each expression-supported _drop_row() (or centralize terminal-drop attribution so exception and missing-result paths share it), and add full-column/from-scratch expression tests asserting the global seed indexes and column. Keep pre-batch and skip.when drops excluded.

Suggestions — Take it or leave it

Document the new public result contract

  • capture_terminal_failures, terminal_failures, and the required early_shutdown completeness check are public caller-facing behavior, but the Fern docs do not mention them. A compact reference/example would help consumers avoid treating a partial list as complete.

What Looks Good

  • Failure capture remains opt-in and O(number of terminal failures), so the default generation path avoids the cost of full tracing.
  • The scheduler correctly waits until retry/salvage exhaustion, preserves the root failed column, expands batch failures to global seed positions, and excludes already dropped rows.
  • The interface consistently exposes the same typed data on normal results, previews, and zero-record errors, including explicit current-invocation resume semantics.

Residual Risk

Focused validation passed: 328 tests, plus Ruff lint and formatting checks. The uncovered expression omission path is exercised by existing integration coverage, but that coverage does not enable or assert terminal-failure capture.

Verdict

Needs changes — the public failure list is incomplete for a required class of terminal row omissions.


This review was generated by an AI assistant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose terminal failed-column and seed-row provenance in public results

2 participants