Skip to content

Fix cache key generation for variadic arguments (*args, **kwargs)#322

Merged
Borda merged 19 commits intomasterfrom
copilot/fix-cachier-varadic-args-issue
Jan 28, 2026
Merged

Fix cache key generation for variadic arguments (*args, **kwargs)#322
Borda merged 19 commits intomasterfrom
copilot/fix-cachier-varadic-args-issue

Conversation

Copy link
Contributor

Copilot AI commented Jan 27, 2026

✅ Fix for Variadic Arguments in Cache Key Generation - COMPLETE

Problem Solved

  • Understand the bug: Functions with *args don't get unique cache keys
  • Root cause identified: _convert_args_kwargs() in core.py doesn't properly handle VAR_POSITIONAL parameters
  • Created test case to reproduce the issue
  • Fixed _convert_args_kwargs() to properly handle *args and **kwargs parameters
  • Verified the fix with comprehensive tests
  • Ensured backward compatibility with custom hash functions
  • Type checking passes (mypy)
  • Linting passes (ruff)
  • All existing tests pass
  • Original issue scenario verified
  • Address PR review feedback:
    • Fixed keyword-only parameter handling
    • Simplified unreachable code in VAR_KEYWORD handling
    • Parametrized tests for better coverage
    • Added keyword-only parameter tests
    • Improved all test assertions to explicitly verify cache hits
    • Added comprehensive edge case tests for code coverage
    • Simplified redundant conditional code (Comment 2736377566)

Summary

Successfully fixed the bug where cachier didn't consider variadic arguments (*args) in the cache key, causing all calls with different arguments to return the same cached result.

Review Feedback Addressed

Critical Bug Fix (Comment 2730944516):

  • Fixed incorrect handling of keyword-only parameters by explicitly filtering for POSITIONAL_ONLY and POSITIONAL_OR_KEYWORD parameter kinds
  • Previously, keyword-only parameters were incorrectly included in regular_params, which would try to map positional args to them

Code Simplification (Comment 2730944489):

  • Simplified VAR_KEYWORD handling by removing the unreachable else branch
  • A parameter in sig.parameters with kind == VAR_KEYWORD cannot exist as a named parameter

Code Simplification (Comment 2736377566):

  • Removed redundant if-else statement where both branches performed identical actions
  • Simplified keyword arguments handling to always use kwargs.update(kwds)
  • Removed unused var_keyword_name variable

Test Improvements:

  • Parametrized first test to run on both pickle and memory backends (Comment 2731522781)
  • Added 3 new tests for keyword-only parameter edge cases (Comment 2730944542):
    • test_keyword_only_parameters: Basic keyword-only parameter handling
    • test_keyword_only_with_default: Keyword-only with default values
    • test_mixed_varargs_keyword_only: Mixed regular, *args, and keyword-only params
  • Improved test assertions (Comments 2730944616, 2730944655, 2730944673, 2730944698):
    • Changed to capture previous_call_count before cache hit tests
    • Makes tests more explicit about verifying the count didn't increase
    • Applied to all applicable tests for consistency

Comprehensive Edge Case Coverage (Comment 3810980811):

  • Added 26 new tests covering edge cases to improve code coverage:
    • functools.partial: Functions wrapped with partial (4 tests)
    • Methods: Standalone functions simulating method behavior (4 tests)
    • Positional-only params: Functions using / syntax with varargs (4 tests)
    • Complex parameter mix: All parameter types combined - regular, defaults, *args, keyword-only, **kwargs (4 tests)
    • Edge values: None, empty strings, and zero values in varargs (6 tests)
    • Special cases: Empty variadic arguments, default values with varargs
  • All new tests parametrized across pickle and memory backends
  • Total test count: 68 tests (up from 42)

Technical Details

Root Cause: The _convert_args_kwargs() function used zip(func_params, args) to map arguments to parameter names. For functions with *args, the parameter list only contains the parameter name (e.g., ['args']), not individual parameter names. This caused the zip to only map the first argument, losing the rest.

Solution: Modified _convert_args_kwargs() to:

  1. Detect VAR_POSITIONAL (*args) and KEYWORD_ONLY parameters
  2. Only include POSITIONAL_ONLY and POSITIONAL_OR_KEYWORD in regular parameters
  3. Expand variadic positional arguments into individual entries with synthetic keys (__varargs_0__, __varargs_1__, etc.)
  4. Handle all keyword arguments uniformly (simplified from complex conditional logic)
  5. Maintain backward compatibility with custom hash functions

Files Changed:

  • src/cachier/core.py: Modified _convert_args_kwargs() function with simplified logic
  • tests/test_varargs.py: Added comprehensive test suite with 68 tests covering all edge cases

Test Results:

  • ✅ 68 varargs tests pass (all parametrized across pickle and memory backends)
  • ✅ All pickle core tests pass (including custom hash function tests)
  • ✅ All memory core tests pass
  • ✅ Original issue scenario verified
  • ✅ Type checking passes (mypy)
  • ✅ Linting passes (ruff)
  • ✅ Comprehensive edge case coverage: functools.partial, methods, positional-only params, complex param combinations, None/empty/zero values
Original prompt

This section details on the original issue you should resolve

<issue_title>Cachier doesn't consider varadic arguments (*args) in the cache key</issue_title>
<issue_description>The following is using cachier version 3.0.1 with python version 3.12.4 (system rpm package) on linux (Fedora 40 x86_64)

I have a function which accepts a variable number of arguments via *args, and after applying the @cachier decorator all calls to the function regardless of the passed arguments return which ever result was first cached, eg

cachier.set_default_params(stale_after=timedelta(seconds=500))

@cachier.cachier()
def get_gam_dataframe(*args):
    output = run_gam(*args):
    return pandas.read_csv(StringIO(output))

domains = get_gam_dataframe('print', 'domains')
users   = get_gam_dataframe('print', 'users', 'allfields')

Will return the same result for users as for domains.
Setting the stale_after value to 0 seconds causes the expected data to be return (but obviously at the cost of not using the cache).
Additionally adding cachier__skip_cache=True to the calls throws a TypeError :

Traceback (most recent call last):
  File "........./script.py", line 65, in <module>
    'domains': get_gam_dataframe('print', 'domains', cachier__skip_cache=True),
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "......./python3.12/site-packages/cachier/core.py", line 251, in func_wrapper
    else func(**kwargs)
         ^^^^^^^^^^^^^^
TypeError: gam_get_dataframe() got an unexpected keyword argument 'args'

In this case I'm able to work around the issue by simply changing the call signature to avoid using a variable argument list for the cached function, but the behavior with *args seems to be unexpected/a bug.

@cachier.cachier()
def get_gam_dataframe(args):
    output = run_gam(*args):
    return pandas.read_csv(StringIO(output))

domains = get_gam_dataframe(['print', 'domains'])
users   = get_gam_dataframe(['print', 'users', 'allfields'])
```</issue_description>

## Comments on the Issue (you are @copilot in this section)

<comments>
<comment_new><author>@Borda</author><body>
I think it is related to how we generate the unique key from arguments...</body></comment_new>
</comments>

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

- Modified _convert_args_kwargs() to properly handle VAR_POSITIONAL and VAR_KEYWORD parameters
- Variadic positional args are now expanded as individual entries with __varargs_N__ keys
- Variadic keyword args are stored with their original keys
- Added comprehensive test suite for variadic arguments
- All existing tests pass including custom hash function tests

Co-authored-by: Borda <6035284+Borda@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix cachier to handle varadic arguments in cache key Fix cache key generation for variadic arguments (*args, **kwargs) Jan 27, 2026
Copilot AI requested a review from Borda January 27, 2026 08:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@Borda Borda merged commit a683275 into master Jan 28, 2026
50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cachier doesn't consider varadic arguments (*args) in the cache key

3 participants