fix(groq): make _combine_llm_outputs None-safe and non-mutating#38759
fix(groq): make _combine_llm_outputs None-safe and non-mutating#38759Andrei Boldyrev (abcgco) wants to merge 4 commits into
Conversation
The merge loop stored the first occurrence of a nested usage dict by reference and then mutated it in place, corrupting the first generation's llm_output before on_llm_end callbacks observe it. Its None-guard was also inverted: a later None silently clobbered an accumulated value, and None followed by a number raised TypeError. Replace with a recursive _update_token_usage (langchain-openai pattern, extended to floats for Groq's timing fields) and skip None values.
Mirror the shape merged in langchain-ai#38751: preserve accumulator-only nested keys, seed first-seen nested dict nodes so they merge instead of colliding with a numeric default, harden token_usage access with .get(), log and pass through unexpected leaf types instead of raising, and type the helper via a recursive TokenUsageTree alias. Groq-specific deltas from the fireworks version: numeric leaves stay int | float (Groq ships float timings such as queue_time / prompt_time), and type mismatches raise TypeError per this package's ruff TRY004.
…llm-outputs # Conflicts: # libs/partners/groq/langchain_groq/chat_models.py # libs/partners/groq/tests/unit_tests/test_chat_models.py
|
Nice fix. The recursive None-safety: Using Type safety: The Edge case handling: The Minor suggestion: Consider adding a test case for the type-mismatch path (e.g., a provider returning Looks good overall. Thanks for the contribution! |
Description:
ChatGroq._combine_llm_outputsmerged nestedtoken_usagedicts by storing the first generation's nested dict by reference and+=-ing into it — mutating the caller'sllm_output, so callbacks/tracing for generation 0 report inflated nested counts (e.g.cached_tokens). A laterNoneclobbered accumulated numbers, andNonefollowed by a number raisedTypeError.This replaces the merge with a recursive
_update_token_usagealigned with the pattern merged for Fireworks in #38751: rebuilds dicts instead of mutating, skipsNonevalues, preserves accumulator-only nested keys, seeds first-seen nesteddictnodes so they merge instead of colliding with a numeric default, hardenstoken_usageaccess with.get(), logs-and-passes-through unexpected leaf types, and types the helper via a recursiveTokenUsageTreealias.Groq deltas from the Fireworks version: numeric leaves are
int | float— Groq ships float timings such asqueue_time/prompt_timeinsidetoken_usage(see the response samples in this module's docstrings) — and type mismatches raiseTypeErrorper this package's ruff TRY004 (Fireworks' config doesn't enforce it).Issue: Fixes #38659
Dependencies: none
Tests: parametrized
None/float handling, an immutability guard (sourcellm_outputunchanged, combined dict not aliased), accumulator-key preservation, first-seen nested-dict seeding, parametrized mismatch raises, unexpected-leaf warn-and-pass-through.pytest tests/unit_tests: 78 passed;ruff check/format,mypy, andlint_importsclean.