Bugfix: Ensure keys in both entities are merged

[estsauver]

I think there's an error here where the entries are not being merged symmetrically. Before the fix, the function only handled keys already present in usage_entry2. Any key that existed solely in usage_entry1 was ignored.

[Copilot]

Pull Request Overview

This PR fixes asymmetric merging in _merge_usage_entries so keys existing only in the first entry are now preserved.

• Adds an else branch to include unseen keys from usage_entry1 during merge
• Introduces a new test to verify that behavior for simple token counts

Reviewed Changes

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

File	Description
dspy/utils/usage_tracker.py	Adds handling for keys present only in the first entry when merging usage dictionaries
tests/utils/test_usage_tracker.py	Adds a test (test_merge_usage_entries_with_new_keys) to ensure unseen keys are preserved

Comments suppressed due to low confidence (2)

dspy/utils/usage_tracker.py:43

• The return type hint dict[str, dict[str, Any]] does not match the actual merged result, which can have non-dict values. Consider updating the annotation to dict[str, Any] for accuracy.

def _merge_usage_entries(self, usage_entry1, usage_entry2) -> dict[str, dict[str, Any]]:

tests/utils/test_usage_tracker.py:162

• Add a test to verify that nested dict values (e.g., prompt_tokens_details and completion_tokens_details) also merge correctly when keys are only in the first entry, covering recursive merge behavior.

def test_merge_usage_entries_with_new_keys():

[chenmoneygithub]

Nice catch, and thanks for the PR! Proposed a simpler way to reduce nesting level

[chenmoneygithub]

probably use the following code to reduce nesting:

        result = dict(usage_entry2)
        for k, v in usage_entry1.items():
            current_v = result.get(k)
            if isinstance(v, dict):
                result[k] = self._merge_usage_entries(current_v, v)
            else:
                result[k] = current_v or 0
                result[k] += v if v else 0
        return result

[estsauver]

Nice edit

[Copilot]

Pull Request Overview

This PR fixes an issue in the usage merger logic so that keys present only in the first usage entry are no longer ignored. Key changes include adding a new test to verify symmetric key merging and updating the _merge_usage_entries function to incorporate keys exclusively available in the first entry.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
tests/utils/test_usage_tracker.py	Added a test case to ensure that unseen keys are merged properly.
dspy/utils/usage_tracker.py	Updated _merge_usage_entries to correctly merge keys from both inputs.

Comments suppressed due to low confidence (1)

dspy/utils/usage_tracker.py:40

• [nitpick] Consider renaming 'current_v' to 'existing_value' or a similar identifier to improve clarity on its role as the value retrieved from usage_entry2.

current_v = result.get(k)