[binaryaaron]

In Rewrite mode, every repair after the first was discarded. The evaluate-repair loop joined each repair result onto the failing rows without overwrite; from the second repair on, the rows already carried _rewritten_text__next, so the join kept the first repair's text. Re-evaluation scored that same text again, _repair_iterations still went up, and max_repair_iterations above 1 (the default is 3) only added model calls.

The fix

One line in src/anonymizer/engine/rewrite/rewrite_workflow.py, mirroring the re-evaluation join below it:

failing_rows = _join_new_columns(failing_rows, repaired, overwrite=True, seed_cols=repair_seed_cols)

overwrite=True lets the repair step's outputs (_leaked_privacy_items, _rewritten_text__next) replace the previous iteration's. seed_cols=repair_seed_cols protects the columns the step only read, which the adapter echoes back serialised (_privacy_qa would come back as a JSON string): the record id, original text, current rewrite, rewrite baseline, _privacy_qa, _privacy_qa_reanswer, leakage_mass, any_high_leaked, utility_score.

Behaviour

One repair: no change. More than one: each repair sees the previous repair's text and re-evaluation scores the new text; a row that passes after its second repair stops with _repair_iterations == 2. For rows repaired more than once, the trace columns _leaked_privacy_items and _rewritten_text__next show the last iteration; column set and dtypes are unchanged. The combined path (Rewrite(use_combined_graph=True)) is unaffected: it writes each iteration to its own columns.

Checks

• make check passes. Full suite: 1,587 passed at f228263b on main (f294dc1e). Earlier validation also covered tests/engine with pandas 3.
• New tests, each with adapter-shaped stubs (a step returns its seed plus only its own columns):
  • test_repair_loop_keeps_each_iterations_repaired_text: three repairs with distinct text, row never passes; final text is the third repair's, counter 3, each later step receives the previous repair's text. Without the fix the final text is the first repair's.
  • test_repair_join_keeps_seed_columns: _privacy_qa seeded as a JSON string stays a dict. Passes before and after; fails if seed_cols is dropped.
  • test_repair_loop_mixed_rows_keep_their_last_repair: rows passing after 0, 1, 2 or never; final text, counter, rows sent to each repair and output order all checked.
  • test_repair_loop_stops_after_successful_repair: passes after the second of three allowed repairs.
  • test_repair_loop_through_real_adapter_keeps_each_repair: through the real NddAdapter with deterministic custom columns, no model requests.
  • test_combined_and_legacy_paths_return_equivalent_repaired_result (strengthened): two distinct repairs; the combined result comes from the real graph finalizer through the real adapter.
• Five of the six fail on previous main (the seed-guard test is meant to pass on both). Removing overwrite=True fails the same five; removing seed_cols fails the seed-guard and real-adapter tests.

Previously stacked on #353; it does not depend on that code and now targets main directly.

🤖 Generated with Claude Code

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

The staged rewrite path now keeps each repair’s latest output while preserving the columns passed into the repair step.

• Rewrite mode uses each repair’s text in the next attempt.

No actionable issues were found. Tests were inspected but not run during this review.

_{Reviews (4) · Last reviewed commit: "fix(rewrite): keep every repair iteratio..." · Reviewed by Greptile}

[binaryaaron]

agent (develop): The repair-loop fixtures now work with #353.

Published as c1d6867fa1e97e9ff921055d762871200e4596e5. The deterministic evaluators now produce an empty _retained_protected_entities list, matching the repair dependency introduced by #353. This fixes the missing-producer failure in test_repair_loop_through_real_adapter_keeps_each_repair when both PRs are combined.

The updated #353 prerequisite was merged without rewriting either branch, and this PR now targets its branch. Merge #353 first. The remaining diff is still the one-line production fix plus its tests.

Validation: 127 focused rewrite/combined/retention tests passed; full make test passed all 1,660 tests; required commit hooks, including make check, passed.

[asteier2026]

I would like to approve the changes in this PR, but it's stacked on PR 353 which I don't think we want.

[binaryaaron]

This PR is now based on main. #353 was closed in favour of #333, and this branch was rebased onto main on 2026-11-02 (head f228263). The diff is the one-line overwrite=True fix plus its tests; all checks pass.

[andreatnvidia]

Looks good to me. Overwriting the repair outputs while preserving the seed columns fixes the repeated-repair behavior and follows the existing re-evaluation pattern.

[andreatnvidia]

One follow-up for the graph line: #319 and #365 still call run_columns(..., "rewrite-repair-...") without overwrite=True, so later repairs still leave the first repair’s text in place. Merging this into #319 conflicts in that loop. Could we carry the overwrite flag and these regression cases into the conflict resolution? Not blocking this PR against main.