Making DependentProductZipperG behave as a compliant ProductZipper

[luketpeterson]

Applying ProductZipper tests to DependentProductZipperG
Expanding ProductZipper tests
Fixing test failures in DependentProductZipperG

[luketpeterson]

@Adam-Vandervorst There were some issues where DependentProductZipperG appeared to be broken. But I don't understand all the situations in which you use it, so I didn't want to make changes that could potentially break behavior without giving you a chance to weigh in.

[Adam-Vandervorst]

The implementation was copy-pasted from the product zipper, so probably just an out-of-sync problem with a fix that wasn't applied to both. I'll run the MORK tests, it's used quite a bit.

[luketpeterson]

No. That was my thought too, at first. But it's actually a mismatch between the product zipper contract and the way it was adapted into the dependent product zipper. I wouldn't have bothered with the pr if it was just some fixed hitting ProductZipperG but not being applied here. I think the changes improve correctness, but I want to make sure you're not relying on the particulars I changed.

…

On Mon, Jul 27, 2026, 8:48 PM Adam V ***@***.***> wrote: *Adam-Vandervorst* left a comment (Adam-Vandervorst/PathMap#56) <#56 (comment)> The implementation was copy-pasted from the product zipper, so probably just an out-of-sync problem with a fix that wasn't applied to both. I'll run the MORK tests, it's used quite a bit. — Reply to this email directly, view it on GitHub <#56?email_source=notifications&email_token=AIY2CNJWSD7MPII5D6QFI7T5HAPAPA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMBZHE3TAMZTGY42M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-5099703369>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIY2CNIQV6XGHKCTVMANJID5HAPAPAVCNFSNUABFKJSXA33TNF2G64TZHM3TQNZVGUZTKNJTHNEXG43VMU5TIOJZGQZTIOBUG44KC5QC> . Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS <https://github.com/notifications/mobile/ios/AIY2CNI2L6R3D23C7HIEJOT5HAPAPA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMBZHE3TAMZTGY42M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG> and Android <https://github.com/notifications/mobile/android/AIY2CNPWDNVP62UMI7C5UC35HAPAPA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMBZHE3TAMZTGY42M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>. Download it today! You are receiving this because you authored the thread.Message ID: ***@***.***>

[Adam-Vandervorst]

Do you have a plain text description of the change?

[luketpeterson]

Do you have a plain text description of the change?

The implementation without the change accumulates (accretes) child zippers in the secondary vec as the zipper moves, because factors that are ascended out of aren't removed during an ascend_until or reset. Ordinary ascend was already fine.

Also the factor_count method is effectively meaningless so I got rid of it.