fix(small-business-loan-agent): empty-string enum schema bug and LLM-judge multi-turn false-block - #2495
Conversation
…ulti-turn false-block
Two bugs found while running the sample end-to-end against the Gemini API:
1. loan_purpose's Literal includes "" as a member so Pydantic emits an empty
string in the generated JSON schema enum. The current google-genai
structured-output validation rejects that outright:
400 INVALID_ARGUMENT: GenerateContentRequest.generation_config.response_schema
.properties[loan_purpose].enum[6]: cannot be empty
Fixed by making the field Optional[Literal[...]] with default=None instead
of including "" as a literal value. No behavior change for downstream code
(missing-field checks treat None the same as they treated "").
2. llm_judge_gate's trajectory check only inspects tool calls made in the
CURRENT invocation (session.events filtered by invocation_id). Since a loan
application is processed across multiple user turns (submit -> approve,
or submit -> pause-for-repair -> resume), any turn that correctly skips a
step completed in an earlier turn gets misjudged as having "skipped
mandatory steps" and the judge blocks the response — even though the
underlying decision was correctly made and persisted. Reproduced on both
the post-approval turn (only LoanDecisionAgent called) and the
post-repair resume turn (skips the already-completed
DocumentExtractionAgent).
Fixed by adding _build_process_history(), which reads the persisted
ProcessStateService state to reconstruct which ALL_STEPS entries were
already completed before this turn, and passing that to the judge prompt
as an explicit "Process History" section distinct from "Tool Call
Sequence (this turn only)". The judge prompt now explains the multi-turn
nature of the workflow and treats a step as a valid skip only when
Process History confirms it was already done.
Verified against both scenarios (approval-only turn, resume-after-repair
turn) with a fresh request ID each time -- previously both were BLOCKED
(trajectory_correct=False), now both are APPROVED (trajectory_correct=True,
grounded_in_context=True), with no regression on the single-turn full
pipeline case.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
We have recently introduced a major restructuring of this repo and we no longer accept receipes under python/agents. Please take a look at the docs to learn more about the new changes and how you can update your code properly and resubmit it under contrib/python: |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| if not process_state: | ||
| return "No process history found for this request (first turn)." | ||
|
|
||
| steps = process_state.get("steps", {}) |
There was a problem hiding this comment.
If the process state dictionary has a 'steps' key that is explicitly set to None, .get('steps', {}) returns None instead of the fallback empty dict. Use process_state.get('steps') or {} to prevent an AttributeError when calling .get() on line 134.
| return "No request ID available — cannot look up process history." | ||
|
|
||
| try: | ||
| process_state = ProcessStateService().get_process_status(request_id) |
There was a problem hiding this comment.
The request_id parameter is used directly in database lookups without validation. If the identifier is user-controlled, this can lead to NoSQL injection or path traversal; please validate and sanitize the input to ensure it matches an expected pattern (e.g., alphanumeric or UUID).
| try: | ||
| process_state = ProcessStateService().get_process_status(request_id) | ||
| except Exception as e: | ||
| return f"Process history unavailable (lookup error: {e})" |
There was a problem hiding this comment.
Returning raw exception messages can expose sensitive system details, internal paths, or database structures. Log the detailed error message internally and return a generic error description to prevent information leakage.
| steps = process_state.get("steps", {}) | ||
| lines = [] | ||
| for step_name in ProcessStateService.ALL_STEPS: | ||
| status = steps.get(step_name, {}).get("status", ProcessStateService.STATUS_NOT_STARTED) |
There was a problem hiding this comment.
If a step's value in the steps dictionary is stored as null in Firestore, steps.get(step_name, {}) will return None instead of an empty dictionary. This will cause an AttributeError when .get("status") is called on it.
happyhuman
left a comment
happyhuman
left a comment
There was a problem hiding this comment.
Please move this recipe to contrib/python before submitting this PR. See docs/ for more info.
2 participants
Summary
Two bugs found while running
small-business-loan-agentend-to-end against the Gemini API key surface:loan_purposeempty-string enum bug.LoanApplicationData.loan_purposeis aLiteral[...]that includes""as a member, so Pydantic's generated JSON schema puts an empty string in the enum. The currentgoogle-genaistructured-output validation rejects that outright:This makes
DocumentExtractionAgentfail on every single request — the sample cannot process any document as shipped. Fixed by making the fieldOptional[Literal[...]]withdefault=Noneinstead of including""as a literal value. No behavior change downstream — the missing-field checks instate_callbacks.pytreatNonethe same way they treated"".LLM-judge gate false-blocks on any multi-turn continuation.
llm_judge_gate's trajectory check (_extract_tool_sequence_and_messages) only inspects tool calls made in the current invocation (session.eventsfiltered byinvocation_id). Since a loan application is processed across multiple user turns by design (submit → approve, or submit → pause-for-repair → resume), any turn that correctly skips a step completed in an earlier turn gets misjudged as "skipped mandatory steps," and the judge blocks the response — even though the underlying decision was correctly made and persisted to Firestore. I reproduced this on both:LoanDecisionAgentis called this turn — Extraction/Underwriting/Pricing ran in the prior turn)DocumentExtractionAgent)Fixed by adding
_build_process_history(), which reads the persistedProcessStateServicestate to reconstruct whichALL_STEPSentries were already completed before this turn, and passes that to the judge prompt as an explicit "Process History" section, distinct from "Tool Call Sequence (this turn only)". The judge prompt now explains the multi-turn nature of the workflow and only treats a step as genuinely missing when Process History confirms it wasn't done in any prior turn either.Verification
Ran the full documented flow (happy path with approval, and pause/repair/resume) end-to-end against the Gemini API with real model calls:
DocumentExtractionAgentnow successfully returns structured output instead of a 400 on every call.trajectory_correct=True, grounded_in_context=Truefrom the judge, with no regression on the single-turn full-pipeline case (extraction → underwriting → pricing all in one turn, which already passed before this change).Notes for reviewers