Skip to content

chore: run detection as the first step of the Rewrite invocation - #346

Merged
binaryaaron merged 3 commits into
aagonzales/graph-sdk/feat/mainfrom
aagonzales/chore/graph-sdk-rewrite-detect-step
Oct 30, 2026
Merged

binaryaaron merged 3 commits into
aagonzales/graph-sdk/feat/mainfrom
aagonzales/chore/graph-sdk-rewrite-detect-step

Conversation

@binaryaaron

@binaryaaron binaryaaron commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

In Rewrite mode, detection is now the first step of the same graph invocation as the rewrite. One protect() call runs detect, then rewrite and assess-final, over every input row. The Replace route and the Rewrite runners share one detect step. Output columns, row sets and DataDesigner calls match the base, and anonymizer.graph.__all__ is unchanged.

This branch stacks on the Rewrite per-target operation PR (#345) and lands on #319's branch by fast-forward after it, without rewriting history.

What changes

  • Shared detect step (engine/detection/detect_step.py). The detect operation moves out of engine/replace/route.py, together with position resolution, accepted-mention extraction and the missing-text check. They become module-level names in engine/detection, and the Replace route imports them and behaves as before.

  • run_with_detection on both Rewrite runners. RewriteWorkflow and CombinedRewriteWorkflow build one target per input row and run detect first. The split on detected entities now happens inside the rewrite step:

    • rows without entities take the passthrough defaults as before;
    • the rest go through the same stages as run (staged or combined);
    • a row the detector drops fails at detect and is absent from the output, as before;
    • when a detector's result cannot be tied to input rows (no reported positions and reordered rows), every target fails at detect, and the stock run rewrites the detection frame as returned.

    RewriteResult carries the detection result and its elapsed time, so the run log is unchanged.

  • Facade. The Rewrite branch of Anonymizer.run and preview calls run_with_detection when the runner's run is the stock one. A runner that overrides run keeps detection followed by that run, as the Replace branch does. The "Running rewrite pipeline" log line now comes after detection.

  • Tests and docs. The shared stub_run_with_detection helper now covers the Rewrite runners. The interface tests that stub a Rewrite runner stub run_with_detection through it, and no assertion changed. AGENTS.md and docs/concepts/graph-sdk.md describe detection as the first step of the Rewrite invocation.

  • Spec program SP-16, five new checks (E16.14-E16.18):

    • detection failures come before rewrite failures in failed_records, on the direct path and on the fallback;
    • position resolution rejects a result whose text differs from its input row when no positions are reported;
    • source_positions include rows without entities;
    • the combined runner records its measurement stage under its own name.

evaluate(), the judges, the kernel and the DataDesigner column configs are unchanged.

Net non-blank, non-comment, non-docstring code lines under src/: +169 (detect_step.py +66, route.py −44, rewrite_workflow.py +102, combined_rewrite_workflow.py +31, anonymizer.py +14).

Known differences

  • If detection or a rewrite step raises, the Rewrite measurement stage records nothing; before, it recorded an entry with status "error". The Replace route already behaves this way.
  • Input in which no row has entities runs one graph invocation, where the old path returned early. It still makes no DataDesigner call.
  • With preview_num_records set, input rows past the limit fail at detect in the graph record and do not appear in failed_records. Output and failed_records are as before.

Checks

  • make check passes.
  • make test: 1,593 passed, with test_wandb_snapshot_rejects_fifo_socket_and_device deselected as on the base.
  • uv run --group dev python -m tests.spec_programs.run_all: 102 pass, 8 retired, 0 fail.
  • Mutation testing of the new code, with the test suite, the spec programs and the parity cases as signals: every mutant is caught except five with no observable difference in output.
  • Rewrite runs make one protect() call when the detector reports positions, and two when an unresolvable result takes the fallback.
  • On both runners, the old path (run after detection) and the new one gave identical output frames, failed records and source positions in each of these cases:
    • every row has entities;
    • the detector drops a middle row, or the first row;
    • the detector reports no positions;
    • the detector reorders rows;
    • no row has entities;
    • a row fails partway through the rewrite.
  • Parity against main (f294dc1e) on pandas 2 and 3, over all cases: no blocking difference. Against the base branch, case by case: no blocking difference. The only informational differences are trace-column order, at the level main shows against itself.
  • Executed lines per case against main: median 1.24×, at most 1.28× on cases whose status matches main (base: 1.23× and 1.26×).
  • No new imports of private names across packages, no new upward imports, and no new import cycles.

🤖 Generated with Claude Code

binaryaaron and others added 3 commits October 29, 2026 01:49
The detect operation, its position resolution, accepted-mention extraction and
missing-text check move out of the Replace route as module-level names so the
Rewrite invocation can share them. The Replace route imports them and behaves
as before.

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
RewriteWorkflow and CombinedRewriteWorkflow gain run_with_detection: one graph
invocation with the detect step first, then rewrite and assess-final over every
input row. The entity split moves inside rewrite, which reads the detection
frame; rows without entities take the passthrough default entry. Results the
detector cannot tie to input rows fail at detect and are rewritten by the stock
run over the detection frame. RewriteResult carries the detection and its
elapsed time so the facade's log block is unchanged.

The facade calls run_with_detection unless the runner overrides run(), in which
case it keeps detect() followed by that run(). Interface tests that stub the
Rewrite runners now stub run_with_detection through the shared helper.

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
…in SP-16

Adds E16.14-E16.18: failure ordering when detection and rewrite both drop rows,
failure ordering on the reroute path, resolve_positions text verification,
source positions covering rows without entities, and the combined workflow's
measurement stage name.

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
@binaryaaron
binaryaaron changed the base branch from aagonzales/chore/graph-sdk-rewrite-lowered to aagonzales/graph-sdk/feat/main October 30, 2026 16:04
@binaryaaron
binaryaaron merged commit f0980f6 into aagonzales/graph-sdk/feat/main Oct 30, 2026
9 checks passed
@binaryaaron
binaryaaron deleted the aagonzales/chore/graph-sdk-rewrite-detect-step branch October 30, 2026 16:04
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.

1 participant