Skip to content

feat(rewrite): add conditional combined workflow - #240

Merged
andreatnvidia merged 8 commits into
mainfrom
andreatnvidia/feat/combined-rewrite-graph
Aug 17, 2026
Merged

feat(rewrite): add conditional combined workflow#240
andreatnvidia merged 8 commits into
mainfrom
andreatnvidia/feat/combined-rewrite-graph

Conversation

@andreatnvidia

@andreatnvidia andreatnvidia commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Related Issue

Closes #237

Plan Document

plans/237/combined-rewrite-graph.md

Summary

Add an opt-in rewrite workflow that executes replacement-map generation, the initial rewrite, evaluation, and a bounded number of conditional repair iterations in one Data Designer graph.

The graph statically unrolls max_repair_iterations and uses SkipConfig.when to skip repair and downstream re-evaluation after each row passes. It supports zero or multiple repair rounds, preserves no-entity passthrough, row order, final metrics, repair counts, and review flags, and continues to execute through NddAdapter.run_workflow().

The legacy workflow remains the default in this PR. Data Designer 0.8 does not expose terminal failed-column and seed-row provenance through its public result API, so a dropped row in the combined graph can only be attributed to rewrite-combined. Draft Data Designer #861 implements the required lightweight provenance API. Keeping this path opt-in avoids reducing default failure observability before that API is merged, released, and integrated.

For entity-bearing rewrite runs, the combined graph reduces the full pipeline's base Data Designer workflow count from five to three. The two detection workflows and the separate non-critical judge path are unchanged.

Experiment evidence

We ran a controlled self-hosted comparison on a single GB300 using GPT-OSS 120B across 12 diverse synthetic PII records. Detection, replacement maps, domain classification, sensitivity decisions, meaning units, and QA plans were generated once and reused by both paths. Both paths received the same initial evaluated rewrite state, execution order was counterbalanced across two six-record groups, and both allowed up to three repair iterations.

Ten records were selected for repair and two were clean skip controls. Both paths made the same repair decision for all 12 records: ten stopped after one repair, two stopped at iteration zero, and none still required repair. The successful run had zero workload failures and zero measured leakage for every output.

Both paths made 80 LLM requests. Across the two groups, legacy used four DD workflows for repair and re-evaluation while combined used two. Combined took 10.56 seconds versus 10.80 seconds for legacy, a 2.2% reduction. This establishes latency parity, not a speedup claim.

Ten of twelve paired outputs were byte-identical and 11/12 final human-review decisions agreed. The paired mean utility delta was -0.0867, the median was zero, and the approximate 95% interval was [-0.2496, 0.0763]. The interval includes zero; separate real-model generations and judge calls remain nondeterministic even with fixed sampling settings.

The experiment also exposed structured-payload checkpoint problems in both paths. The PR now normalizes public QA payloads at combined finalization and at legacy evaluate-to-repair boundaries. Regression tests cover mixed list and NumPy representations across real repair iterations.

Retained GB300 DCGM telemetry showed flat HBM use within both successful controlled runs, with no detectable combined-path GPU-memory regression. Combined Data Designer artifacts were larger: +187% with no repairs and +60% with one repair, although the absolute totals remained below 230 KB for two six-row groups. Retained dataframe memory increased by 6% and 22%, respectively. The five-second telemetry cadence and group-level timings do not support a reliable tail-latency comparison.

An earlier 30-pair Slurm suite remains useful only as scale and stability evidence. Its latency comparison was confounded by different repair behavior and fixed ordering, so it is not used for the performance conclusion above.

Follow-up after Data Designer #861

This PR intentionally stops at an opt-in combined workflow. After Data Designer #861 is merged and available in a supported release, a small Anonymizer follow-up will:

  • capture terminal failure locations through NddAdapter;
  • map seed-row positions to _anonymizer_record_id and failed columns to the existing rewrite stage names;
  • make the combined graph the default;
  • retain the legacy workflow as an explicit fallback during rollout.

Legacy removal can follow production rollout evidence and is not part of this PR.

The branch is synchronized with current Anonymizer main. The combined-runner routing is integrated with main's privacy-goal validation and evaluation refactor.

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, for example fix: handle empty entity list
  • Related issue is linked, or a maintainer-owned no-issue reason is documented above
  • For non-trivial changes, a plan document is linked above, or the no-plan reason is documented above
  • Public API impact checked; skills/anonymizer/SKILL.md updated if needed
  • No real PII added to tests, docs, notebooks, fixtures, or artifacts
  • No API keys, service tokens, private keys, credentials, or real endpoint secrets added

The bundled skill remains on the unchanged legacy default and does not need an update for this opt-in path.

Validation

  • Commands and experiments run:
    • .venv/bin/ruff check --fix .
    • .venv/bin/ruff format .
    • env -u VIRTUAL_ENV .venv/bin/ty check --error-on-warning
    • TMPDIR=<short-temp-path> .venv/bin/pytest --basetemp=<short-temp-path>/pytest - 1,209 passed
    • focused merged interface and rewrite tests - 96 passed
    • focused real Data Designer conditional scheduling with zero, one, two, and exhausted repairs
    • controlled 12-record GB300 repair comparison - 24 paired path executions, 10 repaired and 2 skipped per path, zero workload failures
    • retained GB300 DCGM and artifact analysis - flat HBM, artifact and dataframe overhead quantified
    • earlier paired Slurm scale suite - 60 executions, zero failures
  • Remaining gaps:
    • Default rollout is intentionally deferred until Data Designer #861 is merged, released, and integrated.
    • The real-model comparison supports correctness and latency parity, not a statistically meaningful speedup.

Documentation and Artifacts

  • Docs updated, or not needed
  • If docs changed: make docs-build passes locally
  • If tutorial sources changed: notebooks regenerated with make convert-notebooks
  • If e2e, benchmark, or model-provider behavior changed: relevant validation is listed above

The internal plan documents the rollout decision, benchmark interpretation, completed compatibility coverage, and deferred default integration. No user-facing documentation or tutorial changed.

Refs #237

Signed-off-by: Andre Manoel <amanoel@nvidia.com>
Signed-off-by: Andre Manoel <amanoel@nvidia.com>
Signed-off-by: Andre Manoel <amanoel@nvidia.com>
Signed-off-by: Andre Manoel <amanoel@nvidia.com>
Signed-off-by: Andre Manoel <amanoel@nvidia.com>
Signed-off-by: Andre Manoel <amanoel@nvidia.com>
Signed-off-by: Andre Manoel <amanoel@nvidia.com>
@andreatnvidia
andreatnvidia marked this pull request as ready for review August 12, 2026 18:33
@andreatnvidia
andreatnvidia requested review from a team as code owners August 12, 2026 18:33
@greptile-apps

greptile-apps Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR adds an opt-in combined rewrite workflow that statically unrolls conditional repair and evaluation rounds while retaining the legacy workflow as the default.

  • Adds combined-graph configuration and runner routing.
  • Builds replacement generation, initial rewrite, evaluation, repair, and finalization into one Data Designer workflow.
  • Normalizes evaluation payloads and adds coverage for conditional scheduling, passthrough rows, ordering, failures, and interface routing.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains in the follow-up review scope.

No blocking failure remains.

Important Files Changed

Filename Overview
src/anonymizer/engine/rewrite/combined_rewrite_workflow.py Adds the combined Data Designer graph, conditional repair scheduling, final-state selection, passthrough handling, and result merging.
src/anonymizer/interface/anonymizer.py Routes opt-in rewrite runs through the combined runner while preserving the legacy default.
src/anonymizer/config/anonymizer_config.py Adds the disabled-by-default use_combined_graph rewrite setting.
src/anonymizer/engine/rewrite/rewrite_workflow.py Normalizes structured evaluation payloads at legacy evaluate-to-repair boundaries.
tests/engine/test_combined_rewrite_workflow.py Covers graph construction, row-local conditional repairs, finalization, passthrough behavior, ordering, failure handling, and legacy parity.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Detected rows] --> B{Entities present?}
    B -- No --> P[Apply passthrough defaults]
    B -- Yes --> C[Generate replacement map]
    C --> D[Classify domain and disposition]
    D --> E[Generate QA and initial rewrite]
    E --> F[Evaluate iteration 0]
    F --> G{Needs repair?}
    G -- No --> Z[Finalize latest evaluation state]
    G -- Yes --> H[Repair iteration]
    H --> I[Re-evaluate]
    I --> J{Passed or repair limit reached?}
    J -- No --> H
    J -- Yes --> Z
    P --> M[Merge in original row order]
    Z --> M
Loading

Reviews (2): Last reviewed commit: "refactor(rewrite): clarify combined work..." | Re-trigger Greptile

Comment thread src/anonymizer/config/anonymizer_config.py
Comment thread src/anonymizer/engine/rewrite/combined_rewrite_workflow.py

@lipikaramaswamy lipikaramaswamy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall, excited to see the internal benchmarks - even if there is indeed no improvement in runtime 🙃

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

Copy link
Copy Markdown
Collaborator Author

Additional RAT-Bench throughput results

We ran a paired 100-record-per-split RAT-Bench scorecard using GPT-OSS 120B and identical model, configuration, seed, GPU allocation, and input-token totals.

Split Main input tok/s PR input tok/s Throughput change Slowest-shard wall-time change
diff1 63.8 69.0 +8.2% -7.5%
diff2 67.0 70.1 +4.6% -4.4%
diff3 109.9 116.3 +5.8% -5.5%

All recorded workload and orchestration jobs completed successfully. These results provide consistent throughput evidence across the three RAT-Bench difficulty splits; the controlled paired experiment in the PR description remains the basis for the behavioral-parity and utility conclusions. Detailed run artifacts are retained internally.

@andreatnvidia
andreatnvidia merged commit 3eab7d1 into main Aug 17, 2026
10 of 14 checks passed
@andreatnvidia
andreatnvidia deleted the andreatnvidia/feat/combined-rewrite-graph branch August 17, 2026 14:25
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.

Evaluate DataDesigner-native workflow chaining for Anonymizer pipelines

2 participants