Skip to content

refactor(replace): run detection and replacement as one invocation - #342

Merged
binaryaaron merged 9 commits into
aagonzales/graph-sdk/feat/mainfrom
aagonzales/chore/graph-sdk-detect-replace-one-invocation
Oct 29, 2026
Merged

binaryaaron merged 9 commits into
aagonzales/graph-sdk/feat/mainfrom
aagonzales/chore/graph-sdk-detect-replace-one-invocation

Conversation

@binaryaaron

@binaryaaron binaryaaron commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

In Replace mode, one graph invocation now runs detection and replacement: an invocation-scoped detect step produces each row's mentions, then (for Substitute) the substitute-map step, then replace and verify-patches per row. Output, failed records, errors and log lines match the base; detection's own code, Rewrite, detect-only and evaluate() are unchanged, and no public name is added.

This branch stacks on the Substitute single-invocation PR and is rebased onto its base branch when the stack moves.

What changes

  • The pipeline (engine/replace/route.py). run_replace_pipeline lowers the input frame to one target per row and declares (detect, [substitute-map], replace, verify-patches). The detect step calls the detector once over the whole frame, aligns each returned row to its input row, and returns that row's accepted mentions; a row the detector drops fails at detect and blocks the later steps. verify-patches binds to the detect output by port name, so the kernel is unchanged. Output rows are the detection frame's rows for the targets that reached the end, followed by the replacement columns as before. The Substitute map step is shared with the existing Substitute route.
  • Results the pipeline cannot align. When a caller's detector returns rows that do not map one per input row (for example a duplicated row without reported positions), every target fails at detect, the map step does not run, and replacement runs over the detection frame as returned, through ReplacementWorkflow.run, as before. When positions are not reported, a returned row counts as aligned only if its text equals the input row's text.
  • Rows with missing text. A row whose text is missing keeps the detection frame's own string for it in the replaced column, as before.
  • The runner (engine/replace/replace_runner.py). ReplacementWorkflow.run_with_detection runs the pipeline; ReplacementResult gains optional detection and detection_elapsed fields so the facade can report detection as before. ReplacementWorkflow.run stays for its other callers.
  • The facade (interface/anonymizer.py). Replace mode calls run_with_detection. A caller-supplied replace runner whose class overrides run keeps the previous path: detection, then that run. The Rewrite branch behaves as before; its detection call moved inside the branch with the same arguments and timing points.
  • engine/row_partitioning.py. try_resolve_source_positions returns None instead of raising when a result cannot be aligned to its source.
  • Docs. The Replace-mode pipeline in AGENTS.md shows the single invocation; docs/concepts/graph-sdk.md gains one sentence.
  • Tests. Interface tests that stubbed ReplacementWorkflow.run to drive Replace mode now stub run_with_detection through a shared helper in tests/conftest.py; six assertions name the new method instead of run, with the same conditions.

Net non-blank, non-comment code lines under src/: +173 (route.py +103, replace_runner.py +32, anonymizer.py +30, row_partitioning.py +8).

Known differences

  • With measurement enabled, a Replace run on the default runner no longer records the ReplacementWorkflow.run stage, because that method is no longer called; detection's own stage is still recorded. A runner that overrides run, and the rerouted case above, still record it.
  • Positions a detector reports are trusted as given: output rows come back in input order, and the replaced text is built from the input row's text. Before, a detector that reported positions out of order, or changed the text of a row it reported, saw its own order and text replaced. The built-in detector reports positions in order and does not change text.
  • Substitute without a map workflow raises the same ValueError, now before detection runs instead of after.

Checks

  • make check passes.
  • make test: 1,557 passed. test_wandb_snapshot_rejects_fifo_socket_and_device fails in the test environment with AF_UNIX path too long, as on the base.
  • uv run --group dev python -m tests.spec_programs.run_all: 75 pass, 8 retired, 0 fail.
  • A probe of 20 cases (missing text under each strategy, a detector that duplicates or drops rows, a runner subclass that overrides run, ordinary Redact and Substitute runs) prints status, errors, both result frames cell by cell, failed records and call counts; the output at the base and at the head is identical.
  • Parity against main (f4f2b73d) on pandas 2 and 3, over all cases: no blocking difference beyond the differences the base already has.
  • Executed lines per case against main: median 1.21×, at most 1.24× on cases whose status matches main (base: 1.20× and 1.22×).
  • No new imports of private names across packages, no new upward imports, and no new import cycles.

Main merged (5155d6d)

This branch now includes main at 5155d6d through its base branch, including the Rewrite join change described on the remove-old-sdk PR.

Main merged (f294dc1)

This branch now also includes main at f294dc1 (#328, recovery from incomplete replacements in Rewrite) through its base branch.

🤖 Generated with Claude Code

binaryaaron and others added 9 commits October 27, 2026 13:35
Add run_replace_pipeline: an invocation-scoped detect step calls the
detector once and returns each target's mentions, then the Substitute map
step (when used), replace and verify-patches run per target. A row the
detector drops fails at detect and blocks the later steps. Output rows are
the detection frame's rows, and the failures are the detector's followed by
the map step's. ReplacementWorkflow.run_with_detection exposes it, and
ReplacementResult carries the detection result and its elapsed time.

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Replace mode now calls ReplacementWorkflow.run_with_detection with a
detect callable, so detection runs as the first step of the call that
replaces. Rewrite still runs detection and then its runner. The detection
and replacement log lines keep their text and counts. Interface tests that
stub the replace runner now stub run_with_detection through a shared helper
that calls the detector it is given and returns the stubbed result.

Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
🤖 Generated with Claude Code

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

The single invocation builds each row's target from the input frame's text before detection runs, so a null input became the string 'None' or 'nan' in the replaced column. The replaced cell for a row whose detected text is missing now takes the detection frame's own stringification, as the two-call path did.

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
A detector may return a frame that does not map one row per input row: a duplicated row with no source_positions, or a repeated or out-of-range position list. The single invocation raised 'Workflow result is missing source correspondence' inside the detect step. A result now maps to targets only when it names one distinct input position per returned row, and positions the detector did not report are trusted only when each returned row's text equals the input row's text at that position. Otherwise every target fails at detect and the runner replaces over the detection frame as returned, through the same run() the two-call path used. The detector still runs once and the map step at most once.

🤖 Generated with Claude Code

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

A caller's ReplacementWorkflow subclass may override run(). The single-invocation path called run_with_detection instead, so the override was never reached. When the configured runner's class overrides run(), Replace mode now detects, logs the replacement line and calls self._replace_runner.run() with the detection frame, as before. A runner that does not override run() stays on the single invocation.

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
…de the engine

The replace_method annotation uses the strategy types the module already imports, and the unaligned-result check goes through a row-partitioning helper, so the module no longer imports from the config strategy union or the interface package.

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
…zales/chore/graph-sdk-detect-replace-one-invocation

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
…zales/chore/graph-sdk-detect-replace-one-invocation

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Base automatically changed from aagonzales/chore/graph-sdk-substitute-one-invocation to aagonzales/graph-sdk/feat/main October 29, 2026 21:15
@binaryaaron
binaryaaron merged commit 44924e2 into aagonzales/graph-sdk/feat/main Oct 29, 2026
8 checks passed
@binaryaaron
binaryaaron deleted the aagonzales/chore/graph-sdk-detect-replace-one-invocation branch October 29, 2026 21:15
@binaryaaron

Copy link
Copy Markdown
Collaborator Author

Folded into #319: aagonzales/graph-sdk/feat/main was fast-forwarded to 44924e2e, which contains this branch unchanged, so this step (detection and replacement as one invocation) is now reviewed in #319. Nothing was rewritten; this description stays as the record of the step.

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