Skip to content

Conversation

aditchawdhary
Copy link
Contributor

@aditchawdhary aditchawdhary commented Aug 29, 2025

Purpose

Add json_count_leaves utility function to count leaf elements in nested JSON structures, with integration into multimodal cache system for enhanced debugging capabilities.

Test Plan

# Run the new tests for json_count_leaves function
python -m pytest tests/utils_/test_utils.py::test_json_count_leaves -v

# Run all utils tests to ensure no regressions
python -m pytest tests/utils_/test_utils.py -v

Test Result

  • All new tests pass for various data structures (nested dicts, lists, tuples, mixed types)
  • Edge cases covered (empty structures, single values, deeply nested data)
  • Integration with multimodal cache system works correctly
  • No regressions in existing test suite

Changes made:

  • Added json_count_leaves function in vllm/utils/jsontree.py
  • Enhanced cache debug logging to include leaf count in vllm/multimodal/cache.py
  • Added get_item_complexity method for structural complexity analysis
  • Comprehensive test coverage in tests/utils_/test_utils.py

Example usage:

from vllm.utils.jsontree import json_count_leaves

data = {"images": [{"pixels": [[1, 2], [3, 4]]}]}
count = json_count_leaves(data)  # Returns 4
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

@mergify mergify bot added the multi-modality Related to multi-modality (#4194) label Aug 29, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a json_count_leaves utility function to count leaf elements in nested JSON-like structures and integrates it into the multimodal cache system for enhanced debugging. The implementation is clean and comes with a comprehensive set of unit tests. My main feedback is to improve the robustness of the new json_count_leaves function by switching from a recursive to an iterative implementation. This will prevent potential RecursionError exceptions when dealing with very deeply nested data structures.


def json_count_leaves(value: JSONTree[_T]) -> int:
"""Count the number of leaves in a nested JSON structure."""
return sum(1 for _ in json_iter_leaves(value))
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The current implementation uses json_iter_leaves, which is recursive. This can lead to a RecursionError for deeply nested JSON structures (the default limit in Python is often 1000). To make this utility function more robust, an iterative approach using an explicit stack is recommended. This avoids the recursion limit and is generally safer for functions processing arbitrary tree-like data structures.

Suggested change
return sum(1 for _ in json_iter_leaves(value))
count = 0
stack = [value]
while stack:
node = stack.pop()
if isinstance(node, dict):
stack.extend(node.values())
elif isinstance(node, (list, tuple)):
stack.extend(node)
else:
count += 1
return count

@DarkLight1337
Copy link
Member

Thanks, can you fix pre-commit?

…ested JSON structures, integrated into multimodal cache for enhanced debugging
Signed-off-by: aditchawdhary <[email protected]>
@DarkLight1337 DarkLight1337 enabled auto-merge (squash) August 29, 2025 10:18
@DarkLight1337 DarkLight1337 added the ready ONLY add when PR is ready to merge/full CI is needed label Aug 29, 2025
@vllm-bot vllm-bot merged commit 4f7cde7 into vllm-project:main Aug 29, 2025
43 of 46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
multi-modality Related to multi-modality (#4194) ready ONLY add when PR is ready to merge/full CI is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants