fix(types): add top_logprobs to LogprobsPart to match API response shape#450
Open
xodn348 wants to merge 1 commit into
Open
fix(types): add top_logprobs to LogprobsPart to match API response shape#450xodn348 wants to merge 1 commit into
xodn348 wants to merge 1 commit into
Conversation
The Together API returns `top_logprobs` as a List[Dict[str, float]] (one dict per token mapping token string to log-probability), but LogprobsPart had no field for it. Since BaseModel uses extra="allow", the data was stored as an extra field with no declared type, causing: - Missing type information for static analysis / IDE autocompletion - PydanticSerializationUnexpectedValue warnings on model_dump() Add `top_logprobs: List[Dict[str, float]] | None = None` to LogprobsPart and add three unit tests covering the list round-trip, warning-free serialization, and the default-None path. Fixes togethercomputer#443 Signed-off-by: xodn348 <xodn348@tamu.edu>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Have you read the Contributing Guidelines?
Issue #443
Describe your changes
LogprobsPart(insrc/together/types/common.py) was missing a typed field fortop_logprobs, even though the Together API returns it alongsidetokensandtoken_logprobswhenlogprobs=Nis requested.Because
BaseModelusesextra="allow", the data was silently stored as an untyped extra attribute. Two problems result:response.choices[0].logprobs.top_logprobsasAny, losing all type safety.PydanticSerializationUnexpectedValuewarnings onmodel_dump()— when serializing, Pydantic sees alistvalue for a field that has no declared schema and emits a warning.The fix adds:
to
LogprobsPart, matching the actual API shape: onedictper token mapping each candidate token string to its log-probability.Three unit tests are added to
tests/unit/test_logprobs_type.py:top_logprobsis parsed aslist[dict]model_dump()with warnings-as-errors enabledNonewhen field is absentSummary
Added
top_logprobs: List[Dict[str, float]] | None = NonetoLogprobsPartso that the field is fully typed, IDE-visible, and serializes without Pydantic warnings. Three regression tests cover the happy path, warning-free serialization, and the optional-field default.Issue
Fixes #443
Local verification
Risk
Purely additive type annotation — no runtime behaviour change. The field defaults to
None, so all existing callers that don't usetop_logprobsare unaffected. Users who do accessresponse.choices[0].logprobs.top_logprobsalready receive the correct list value (stored viaextra="allow"); this PR simply makes the field explicit and statically typed.