Skip to content

fix(cache): make multi-split document IDs unique - #1368

Open
linhongyu510 wants to merge 1 commit into
huggingface:mainfrom
linhongyu510:fix/1363-multi-split-doc-ids
Open

fix(cache): make multi-split document IDs unique#1368
linhongyu510 wants to merge 1 commit into
huggingface:mainfrom
linhongyu510:fix/1363-multi-split-doc-ids

Conversation

@linhongyu510

Copy link
Copy Markdown

Summary

Fixes #1363.

LightevalTask._get_docs_from_split() currently restarts enumerate() for every evaluation split and assigns each document the numeric-string index. Multi-split tasks therefore produce duplicate IDs such as ["0", "1", "0"]. SampleCache uses that ID as its per-task Parquet key, so collisions can crash cache reads or silently return one split's response for another split's document.

This change prefixes the positional ID with the split name only when more than one split is requested:

  • multi split: validation:0, validation:1, test:0
  • single split: unchanged numeric IDs such as 0

Compatibility

Single-split tasks keep their existing IDs and cache keys. Existing multi-split numeric cache rows no longer match the new split-aware IDs, so affected samples are recomputed once instead of being silently rebound. New split-aware rows can coexist with old rows and round-trip independently.

This focused fix does not add duplicate-ID validation inside SampleCache; that broader defensive policy remains open for maintainer guidance on #1363.

Duplicate-work check

I searched open PRs by issue number, doc.id, split:index, multi-split cache, and sample-ID collision keywords. No open PR addresses #1363 or this collision.

Tests

Baseline on origin/main:

  • the regression test fails with ['0', '1', '0']

After this change:

  • uv run pytest tests/unit/tasks/test_lighteval_task.py -q: 4 passed
  • the regression covers multi-split uniqueness, single-split compatibility, a legacy numeric Parquet cache miss, split-aware cache writes, and round-trip reads
  • uv run ruff check src/lighteval/tasks/lighteval_task.py tests/unit/tasks/test_lighteval_task.py: passed
  • uv run ruff format --check src/lighteval/tasks/lighteval_task.py tests/unit/tasks/test_lighteval_task.py: passed
  • git diff --check: passed

An expanded cache-suite run produced 11 passed and 1 skipped; six test_cache_decorator_presence subtests require the optional vLLM backend and fail during placeholder attribute access on this macOS environment. They do not exercise the changed ID or cache path.

AI assistance

AI assistance was used for investigation, implementation, test generation, and drafting this description. I reviewed the diff and ran every command reported above.

@linhongyu510
linhongyu510 marked this pull request as ready for review August 30, 2026 03:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sample cache: doc.id collides across evaluation splits, crashing runs and silently serving one split's answers for another split's questions

1 participant