Repository navigation
refactor: remove legacy DD sync model paths - #359
Conversation
Signed-off-by: Andre Manoel <amanoel@nvidia.com>
[Medium impact] The PR appears safe to merge; no new actionable issues were found. SummaryThe PR removes legacy synchronous detection paths and changes rewrite evaluation and repair to await model calls.
No new actionable issues were found in the latest changes. Reviews (4) · Last reviewed commit: "test(rewrite): cover async failures for ..." · Reviewed by Greptile |
Signed-off-by: Andre Manoel <amanoel@nvidia.com>
Signed-off-by: Andre Manoel <amanoel@nvidia.com>
Signed-off-by: Andre Manoel <amanoel@nvidia.com>
lipikaramaswamy
left a comment
There was a problem hiding this comment.
Approved after reviewing the current head, 373b5bb312ae22f48a9eaa6281a9f5c56eb37c82.
Review
No actionable correctness issues found. The chunked-validation plugin now uses the existing async path exclusively and rejects direct synchronous calls before model dispatch. The additional rewrite changes consistently await the quality re-answer, privacy re-answer, quality comparison, and repair model calls; the combined graph awaits these generators, and trace context remains active across awaits.
Validation on the current head
Independently ran:
PYTHONPATH=<isolated PR source> python -m pytest tests/engine tests/interface tests/test_measurement.py -q --tb=short
Result: 1,101 passed. Also reviewed the new standalone/combined dispatcher coverage, timeout and cancellation propagation tests, and sync/async trace-attribution coverage. git diff --check passed for the follow-up changes.
Live integration evidence
A full public Anonymizer.preview() rewrite workflow passed on the earlier revision 9e103e3e33020a32ffe62dc0e8f4d87689c18242, using live GLiNER2 and the default OpenRouter model selections:
- Two entirely fabricated input records, both returned; zero failed records.
- Validation limited to two candidates per chunk: five chunks started and completed, with five concurrently active; zero synchronous chunked-validation plugin calls.
- The rewrite workflow completed in 125.31 seconds, produced the expected rewrite and metric columns, and preserved the no-PII record unchanged.
- Evaluation passed at iteration 0, so this live run did not exercise repair.
This live evidence covers the earlier detection cleanup; the subsequent rewrite/trace changes were reviewed and tested on the current head as described above, but were not rerun against live providers.
Merge coordination
Please coordinate with #339 and #305 (and its dependent #265): they modify the removed sync validation code/tests, and #305 adds an augmentation sync entry point using the deleted bridge. Keep their intended behavior in the async paths when resolving overlaps.
DataDesigner removed its sync engine in NVIDIA-NeMo/DataDesigner#767, but Anonymizer retained a duplicate sync detection path and four synchronous model calls in rewrite evaluation and repair. These calls now use
await model.agenerate()directly on DD's async loop, with trace attribution preserved across awaits.Related Issue
No linked issue required: maintainer-owned cleanup following DataDesigner's sync engine removal.
Plan Document
No plan required: internal engine migration to existing DD async execution, preserving the public API and pipeline logic.
Summary
Type of Change
Contributor Checklist
Validation
TMPDIR=/private/tmp .venv/bin/pytest -q --tb=short --basetemp=/private/tmp/a359-pytest-short: 1,605 passed. The short temporary path avoids macOS Unix-socket path constraints.fea5a60with this PR through a scripted HTTP provider: six scenarios in both standalone and combined rewrite modes (12 paired comparisons). Complete public outputs, internal trace dataframes, request counts, failed records, correction attempts, and trace column coverage matched.373b5bb: no actionable findings..venv/bin/ruff check --fix . && .venv/bin/ruff format .VIRTUAL_ENV="$PWD/.venv" .venv/bin/ty check --error-on-warning.venv/bin/python tools/codestyle/copyright_fixer.py --check .uv lock --checkandgit diff --checkSuccessful async traces were preserved; the old failed synchronous-probe traces disappeared. Full detection/rewrite end-to-end comparisons used the scripted provider; live coverage focused on evaluation and repair.
The format, copyright, and aggregate commit-hook wrappers were skipped after running their equivalent checks directly because those wrappers invoke
uv run. Remaining applicable hooks and DCO signoff passed.Documentation and Artifacts