Repository navigation
docs(replace): state when a shared substitute can differ - #399
Open
binaryaaron wants to merge 2 commits into
Open
binaryaaron wants to merge 2 commits into
binaryaaron wants to merge 2 commits into
Conversation
"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>
Contributor
[Low impact] This PR appears safe to merge. SummaryClarifies when
No actionable issues found. Tests were inspected, not run.
Reviews (1) · Last reviewed commit: "test(replace): pin the newcomer-first fo..." · Reviewed by Greptile |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
consistencydocs row and theSubstitute.consistencyfield 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,consistencyrow: ""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.consistencydescription: the same rule, in one sentence for each part._fold_replacement_mapsdocstring: 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:
Alice → Maya,Bob → MayaandALICE → NORA,Alicekeeps Maya andBobgets 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_mapspin 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 givesInitech→[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 givesBob→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