Fix LSET listpack conversion after element replacement

[Taeknology]

Issue

LSET replaces an existing list element, but the old conversion check treated the new value like an appended element. With list-max-listpack-size 3, replacing one small value in a 3-element listpack could wrongly convert it to quicklist.

Fixes #2162.

Changes

• Run the encoding conversion check after LSET replaces the element.
• Use the original encoding to choose the conversion direction:
  • listpack: check if it should grow to quicklist
  • quicklist: check if it can shrink to listpack
• Add tests for small and large replacements.

Tests

• make -C src valkey-server -j$(nproc)
• ./runtest --single tests/unit/type/list.tcl (291 passed, 0 failed)
• git diff --check
• Manual edge-case checks for negative indexes, out-of-range indexes, large values, and quicklist-to-listpack shrinking.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 52c4365c-9865-4bb8-b94e-5d5eabbaa306

📥 Commits

Reviewing files that changed from the base of the PR and between 699c64e and 67e0a28.

📒 Files selected for processing (2)

• src/t_list.c
• tests/unit/type/list.tcl

---

📝 Walkthrough

Walkthrough

lsetCommand in src/t_list.c is changed to snapshot the list encoding before calling listTypeReplaceAtIndex and then select LIST_CONV_GROWING or LIST_CONV_SHRINKING accordingly, replacing the prior append-style pre-conversion call. Two new tests and one updated test in tests/unit/type/list.tcl cover the corrected behavior.

Changes

LSET encoding conversion fix

Layer / File(s)	Summary
lsetCommand encoding conversion logic src/t_list.c	Captures old_encoding before listTypeReplaceAtIndex and dispatches listTypeTryConversion with LIST_CONV_GROWING when the prior encoding was listpack, or LIST_CONV_SHRINKING otherwise. Removes the previous growing-conversion call that ran before the replacement via an append-style helper.
LSET encoding tests tests/unit/type/list.tcl	Adds two new tests asserting that LSET preserves listpack when the replacement fits within list-max-listpack-size and converts to quicklist when it does not. Updates the existing listpack→quicklist conversion test's LSET assertion to be conditional on $max_lp_size rather than unconditionally expecting quicklist.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title clearly summarizes the main change: fixing LSET listpack conversion after element replacement, which is the primary focus of the changeset.
Description check	✅ Passed	Description is well-related to the changeset, explaining the bug, root cause, the fix implementation, and testing performed.
Linked Issues check	✅ Passed	The changes fully address issue #2162 by fixing the LSET encoding conversion logic to only convert when necessary, with new tests validating both small and large value replacements.
Out of Scope Changes check	✅ Passed	All changes in src/t_list.c and tests/unit/type/list.tcl are directly related to fixing the LSET listpack conversion bug described in the linked issue.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.71%. Comparing base (699c64e) to head (67e0a28).
⚠️ Report is 33 commits behind head on unstable.

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #3985      +/-   ##
============================================
+ Coverage     76.63%   76.71%   +0.08%     
============================================
  Files           162      162              
  Lines         80788    80790       +2     
============================================
+ Hits          61909    61978      +69     
+ Misses        18879    18812      -67     

Files with missing lines	Coverage Δ
src/t_list.c	93.11% <100.00%> (+0.02%)	⬆️

... and 25 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.