Skip to content

docs(replace): state when a shared substitute can differ - #399

Open
binaryaaron wants to merge 2 commits into
aagonzales/graph-sdk/feat/mainfrom
aagonzales/docs/run-consistency-fallback-wording
Open

binaryaaron wants to merge 2 commits into
aagonzales/graph-sdk/feat/mainfrom
aagonzales/docs/run-consistency-fallback-wording

Conversation

@binaryaaron

Copy link
Copy Markdown
Collaborator

The consistency docs row and the Substitute.consistency field description now say that a mention can get a different substitute from the one its value shares. This happens where the shared substitute would clash with another value, or another value's substitute, in the same record. Until now both texts said each value gets one substitute within its record ("record") or across the whole run ("run"), and neither holds in those cases. The fold's docstring now also describes both orders in which a run-wide substitute and another key's proposal can meet in one record, and two new test rows pin them. Code behaviour does not change. This covers findings 1 to 3 of the post-merge review on #394.

What changes

  • docs/concepts/replace.md, consistency row: ""record" shares each value's substitute within its record; "run" shares it across the whole run. A mention can still get a different substitute where the shared one would clash with another value, or another value's substitute, in the same record."
  • Substitute.consistency description: the same rule, in one sentence for each part.
  • _fold_replacement_maps docstring: it said that when another key proposes the holder's run-wide substitute, the proposer always falls back. That holds when the holder comes first in the record. When the proposer comes first and the holder's run-wide substitute collides in that record, the proposer keeps the value and the holder falls back to a placeholder. The docstring is also rewrapped to 120 characters.

Review focus

Two examples of a different substitute, both older than #394. This PR documents them and does not change them:

  • Run mode: Alice gets Maya in record 1. Record 2 also contains Maya as a value, so there Alice gets her own proposal (Nora). In record 3 she gets Maya again. A join on the substitute misses record 2.
  • Either mode: in a record with Alice → Maya, Bob → Maya and ALICE → NORA, Alice keeps Maya and Bob gets a placeholder. ALICE, the same value in different case, gets NORA, because Bob was also proposed Maya.

Testing

Two new rows in test_fold_replacement_maps pin current behaviour, so both pass before and after this change. Each has a mutation that makes it fail:

  • newcomer-first: [Acme→Nova], then [Initech→Nova, Acme→Zeta]. Run mode gives Initech→[SUBSTITUTE_COMPANY_1], Acme→Nova, and record mode leaves the input unchanged. Removing the run-wide check against substitutes other keys already received makes this row fail.
  • holder-fallback-after-proposer: [Alice→Maya], then [Bob→Nora, Alice→Nora, Maya→Lena], which gives Bob→Nora, Alice→[SUBSTITUTE_FIRST_NAME_1], Maya→Lena. Letting the holder's fallback skip the check against values already emitted in the record makes this row fail.

🤖 Generated with Claude Code

"record" and "run" consistency share a value's substitute within its record
or across the run, but a mention can still get a different substitute where
the shared one would clash with another value, or another value's
substitute, in the same record. Two cases: a record that itself contains the
run-wide substitute as a value, and a later spelling variant whose shared
substitute another value in the record also proposed. Say so in the
consistency docs row and the field description, and describe in the fold
docstring how a run-wide substitute and a colliding fallback are ordered
against other keys in a record.

Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Add fold cases where a new key proposes a run-wide substitute before its holder, and where the holder falls back after another key already emitted its fallback value.

Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
@binaryaaron
binaryaaron requested a review from a team as a code owner November 10, 2026 17:55
@greptile-apps

greptile-apps Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] This PR appears safe to merge.

Summary

Clarifies when Substitute.consistency can give a mention a different substitute.

  • Substitute consistency descriptions now explain when shared names can differ.

No actionable issues found. Tests were inspected, not run.

binaryaaron explicitly identifies collision fallbacks that break joins as existing behavior being documented, not changed.

Reviews (1) · Last reviewed commit: "test(replace): pin the newcomer-first fo..." · Reviewed by Greptile

This branch has not been deployed

No deployments
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