[binaryaaron]

With Substitute(consistency="run"), two different values no longer receive the same substitute anywhere in the run, and a value that already has a run-wide substitute keeps it. Before this change, each record checked collisions only against itself. As a result, "Alice" in one record and "Carl" in a later record could both become "Maya", and a join on substitutes could merge two people. The docs recommend run mode for exactly that join.

Behaviour (run mode only)

• A substitute emitted for one value anywhere earlier in the run collides for every other value. The later value falls back to its own proposal, then to a placeholder.
• Placeholders are numbered across the run, so a token such as [SUBSTITUTE_FIRST_NAME_1] names one value only.
• A value that already holds a run-wide substitute keeps it when another value in a later record proposes the same substitute, in either order within the record; the proposer falls back. Before, the established value lost its substitute for that record and the newcomer took it.
• Unchanged: a substitute that equals a protected value in its record still falls back, and within a record the first value keeps a contested substitute.

consistency="record" output is unchanged. The Rewrite path uses record mode and is unaffected.

The run-mode join recommendation in docs/concepts/replace.md stays, because the uniqueness it needs now holds; one sentence there states the guarantee. The fold's docstring describes the run-mode rules.

Review focus

• The collision test case changes its run-mode expectation: record 2 keeps Acme->Nova from record 1, and Initech falls back to a placeholder. The old expectation pinned the precedence this PR reverses.
• A value that holds a run-wide substitute ignores other values' proposals even in a record where it uses its own fallback (for example because its run-wide substitute equals a protected value there). Its case variants in that record then share the fallback; before, a variant could fall back separately.
• Limit: uniqueness is checked against the values each record protects. If a later record contains an original equal to an earlier substitute, the run-wide value falls back in that record, as any substitute that equals a protected value does.

Testing

• tests/engine/test_llm_replace_workflow.py: 3 failed, 22 passed before this change; 25 passed after. The failing cases are the flipped collision case, a new cross-key case (one value in record 1 and a different value in record 2 propose the same substitute), and the property test's run case, which now asserts that each substitute maps to one value across all 300 random records. Before this change, the property test fails on a placeholder token shared by several values.
• Record mode: on the property test's 300 random maps, record-mode output is byte-identical before and after this change.
• Placeholder generation checks the run-wide substitutes in place instead of copying them for each placeholder. A run-mode fold of 20,000 records that needs a placeholder in each record takes 0.38 s instead of 29.6 s, and the output is unchanged.

🤖 Generated with Claude Code

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; the earlier copying issue is fixed and no new actionable issues were found.

Summary

Run mode now reserves emitted substitutes across records, numbers placeholders across the run, and gives established mappings priority over new proposals.

• Run mode gives different values different substitutes across records.

_{Reviews (2) · Last reviewed commit: "perf(replace): check run-wide substitute..." · Reviewed by Greptile}

[greptile-apps]

Placeholders repeatedly copy earlier substitutes

Each placeholder copies every earlier substitute through | taken.keys(). For distinct names that repeatedly receive the same proposal, almost every record needs a placeholder. Doubling such a run can roughly quadruple this copying work.

Check each candidate against taken separately instead of copying it into protected.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[binaryaaron]

agent (review-pr): Post-merge review of #394, checked at PR head 259492a (merged as c3798fe). Findings 1 to 3 and 5 were re-checked against the current line head, 5b764ca, after #397: all four still apply.

Verdict: approve. The change does what the PR describes: run-wide uniqueness of substitutes, an established mapping wins, record mode unchanged, deterministic output, and linear time. None of the findings below needed to block the merge. They are follow-ups.

Verification

Each item was checked with direct calls to _fold_replacement_maps at the PR head and at the previous line head.

Requirement	Result
Uniqueness across the run	[Alice→Maya] then [Carl→Maya] gives Carl→[SUBSTITUTE_FIRST_NAME_1] (before the PR: Carl→Maya). In a fuzz of 200 runs of 40 records each, including requested values and originals shaped like placeholders, every folded substitute maps to exactly one original. Before the PR, two different values could share a [SUBSTITUTE_P_n] token.
An established mapping wins	When record 2 is [Alice→Nora, Bob→Maya], Alice keeps Maya and Bob gets a placeholder, in either order within the record. taken covers the proposer-first order and the key not in run_wide exclusion covers the holder-first order.
Placeholders numbered across the run	run_placeholders carries the counter: _1 in record 1, _2 in record 2.
No change outside record mode	taken and run_wide are filled only when consistency == "run", so in record mode key not in run_wide is always true and taken.get is always empty. Record-mode output is identical before and after. Rewrite always folds in record mode.
Determinism	The new sets are used only for membership tests, so output does not depend on set iteration order.
Performance	One placeholder per record in run mode: 0.10 / 0.16 / 0.37 s at 5k / 10k / 20k records. The PR's first commit (d8a8b56) took 1.6 / 6.3 / 29.7 s, which is quadratic.

Findings

1. One value can still get more than one substitute across the run, and the docs say otherwise (pre-existing). If a key's run-wide substitute equals a protected value in a later record, the key falls back for that record. Alice can get Maya, Nora, Maya in records 1, 2, 3 when record 2 contains an original "Maya". A join on the substitute then misses Alice's record 2. docs/concepts/replace.md says "run" gives each value "one substitute across the whole run". The sentence added about two values never sharing a substitute covers only the other direction. One clause would close the gap, for example "except in a record where that substitute is itself a protected value".
2. The docstring on the fold overstates proposer fallback (llm_replace_workflow.py, the paragraph ending "so the proposer falls back"). It holds only when the established value comes first in the record. With run-wide Alice→Maya and record 2 = [Bob→Nora, Alice→Nora, Maya→Lena], Bob keeps Nora and Alice gets the placeholder. Uniqueness holds; only the wording promises more than the code does.
3. No test pins the proposer-first order. _COLLISION and _COLLISION_RUN in tests/engine/test_llm_replace_workflow.py put the established value first in record 2. Suggested fixture: _maps([("company","Acme","Nova")], [("company","Initech","Nova"), ("company","Acme","Zeta")]), with the run-mode expectation Initech→[SUBSTITUTE_COMPANY_1] and Acme→Nova.
4. Minor: the placeholder counter can fall behind the issued index. When _collision_safe_synthetic skips an index that is already protected, it returns a higher index, but placeholders[label] keeps the lower value, so the next placeholder of that label probes the same protected token again. Output stays correct; the cost is one extra probe per skipped index.
5. Merge note. fix(replace): keep run-mode substitutes unique across the run #394 landed as a merge commit, so the quadratic first commit (d8a8b56) stays in line history. git bisect over that range passes through it.

Interaction with other PRs

#333 and #379 do not interact with this change. Both target main, and both touch only the Rewrite path. The Rewrite replacement map folds in "record" mode, so the run-mode bookkeeping added here never reaches it. #333 conflicts textually with the line in combined_rewrite_workflow.py when ported, not in behavior.