[draft] discuss: ListProxy.reverse patch minimality

[berendkleinhaneveld]

Summary

ListProxy.reverse() currently emits up to N replace patches for an N-element list. The original code comment frames this as 2× the patches needed (with positions i and N-1-i treated as a swap pair). This PR pins the current semantics with tests and asks: keep, change, or close?

This PR is tests only — no production behavior change — to keep the discussion focused on the design call.

Patch count, current vs. alternatives

Shape	Patches for N=4 (distinct values)	Patches for N=5	Round-trips?
Current: range(N), single replace per slot	4	4 (middle no-op filtered)	yes
Plan sketch: range(N // 2), two replaces per swap	4	4	yes
"Drop half" hypothetical	2	2	no

The "drop half" row is the one the existing in-code comment seems to imagine, but I couldn't make it work. Quick proof on master:

from patchdiff import apply
from patchdiff.pointer import Pointer
base = [1, 2, 3, 4]
half = [
    {"op": "replace", "path": Pointer([0]), "value": 4},
    {"op": "replace", "path": Pointer([1]), "value": 3},
]
apply(base, half)  # -> [4, 3, 3, 4], not [4, 3, 2, 1]

swap = [
    {"op": "replace", "path": Pointer([0]), "value": 4},
    {"op": "replace", "path": Pointer([3]), "value": 1},
]
apply(base, swap)  # -> [4, 2, 3, 1], not [4, 3, 2, 1]

With replace semantics each destination slot needs its own patch — there's no way to make positions i and N-1-i cover for each other. RFC 6902 has move, which could swap with two moves via a scratch slot, but apply.py here doesn't implement move and the plan explicitly excludes new op types.

So the realistic options are both N patches, just in different iteration orders. The plan's "minimal" sketch (range(N // 2) with two record_replace calls per iteration) ends up emitting the same count — the parenthetical in the plan acknowledges this: "Both positions in the swap need a patch — you can't represent the swap with one replace because the value at j is the missing piece for the patch at i."

Consequences for apply / memory

Since the patch count is unchanged regardless of which loop shape we pick, there's no measurable apply runtime or memory delta between the two iteration styles. The forward and reverse patch lists are the same length and touch the same slots.

What this PR does

• Adds test_list_reverse_emits_n_patches_for_even_n — pins N patches for N=4 with distinct values.
• Adds test_list_reverse_emits_n_minus_one_patches_for_odd_n — documents that the odd-N middle is filtered to a no-op by record_replace.
• Adds test_list_reverse_round_trip parametrized over n ∈ {0, 1, 2, 3, 4, 5, 10} — forward and reverse both round-trip.
• Imports apply in the test module.

No changes to patchdiff/produce.py.

Question for the author

Three reasonable directions:

1. Land these tests, keep current behavior. The "redundancy" framing in the existing code comment is misleading — N patches is the floor for replace-only output, and the current loop already hits it. The pinning tests make that explicit.
2. Refactor the loop to range(N // 2) with two emits per iteration. Same patch count, arguably more readable ("reverse is N//2 swaps"). Cosmetic.
3. Close as won't-fix. Drop the misleading framing from any code comments, leave the loop as-is.

My weak recommendation is (1): the tests are worth keeping either way, and they document the floor so future readers don't go chasing the same "redundancy" thread.

Land minimal, keep current, or close as won't-fix?

Test plan

• uv run pytest tests/test_produce_list.py -v — 84 passed
• uv run pytest -x -q — 277 passed
• uv run ruff check — clean
• uv run ruff format --check — clean

🤖 Generated with Claude Code

[berendkleinhaneveld]

Closing as won't-fix per author decision.

Recap of the analysis (full detail in the PR body):

• The original code comment framed positions i and n-1-i as a redundant swap pair, but with replace semantics each slot needs its own patch — N patches is the floor, not 2× the minimum.
• The plan's range(N // 2) sketch ends up emitting the same N patches, just in a different order.
• No misleading comments in produce.py itself to clean up — the framing only appeared in the plan.

Leaving the implementation as-is. Branch will be deleted.