PG1204 commented on code in PR #8617:
URL: https://github.com/apache/texera/pull/8617#discussion_r4068216347
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/huggingFace/codegen/QaRankingCodegen.scala:
##########
@@ -79,18 +79,18 @@ object QaRankingCodegen extends TaskCodegen {
| if isinstance(body, dict):
| # Third-party chat providers answer via
choices[0].message;
| # hf-inference returns the native {"answer": ...}
shape.
- | if "choices" in body:
- | return body["choices"][0]["message"]["content"]
+ | if body.get("choices"):
+ | return body["choices"][0].get("message",
{}).get("content", json.dumps(body))
| return body.get("answer", json.dumps(body))
| return json.dumps(body)
| elif task == "table-question-answering":
| if isinstance(body, dict):
- | if "choices" in body:
- | return body["choices"][0]["message"]["content"]
+ | if body.get("choices"):
+ | return body["choices"][0].get("message",
{}).get("content", json.dumps(body))
| return body.get("answer", json.dumps(body))
| return json.dumps(body)
| elif task in ("zero-shot-classification",
"sentence-similarity", "text-ranking"):
- | if isinstance(body, dict) and "choices" in body:
- | return body["choices"][0]["message"]["content"]
+ | if isinstance(body, dict) and body.get("choices"):
+ | return body["choices"][0].get("message",
{}).get("content", json.dumps(body))
Review Comment:
Confirmed, and there were two more: {"choices": {...}} raises KeyError: 0
and {"choices": [42]} raises AttributeError. Two shapes were also silently
wrong rather than raising a null content returned Python None into the cell,
and a list-valued content returned the raw list.
Worth flagging what your comment uncovered: _parse_response already catches
(KeyError, IndexError, TypeError), so on main these shapes were degrading to
raw JSON correctly. Moving to .get chaining changed the exception type to
AttributeError, which that handler doesn't catch, so this PR was introducing
the escape, not just failing to fix it. Good catch.
Rather than repeat type checks at each site, I added a shared
_chat_message_content(body) helper to HuggingFaceCodegenBase that validates
every level and returns None when the body isn't a readable chat response. All
four extractions here and in TextGenCodegen go through it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
