fix(checks): join list[str] context in Groundedness/Contradiction judges instead of str()-ing it 🤖🤖🤖🤖#2626
Conversation
…ges instead of str()-ing it
Both judges declare `context: str | list[str] | MISSING` and document passing
a list of context documents (see the Groundedness docstring example), but
`get_inputs()` rendered that value with a bare `str(...)` call. For a list,
`str()` produces Python's repr -- brackets, comma separators, and quoted
(sometimes backslash-escaped, when an item contains an apostrophe) items --
which was injected verbatim into the `{{ context }}` slot of the judge
prompt templates. An empty list rendered as the literal string "[]" instead
of empty context. This was previously the *tested* behavior: several
existing tests asserted the raw repr, e.g.
`result.details["inputs"]["context"] == "['Direct context']"`.
Add a small `format_prompt_text()` helper in judges/base.py that joins list
values into a newline-separated block (matching the `"\n".join(...)`
convention already used elsewhere in this codebase, e.g.
translators/google_response.py and export/junit.py) and falls back to
`str(value)` for everything else (plain strings, NoMatch, arbitrary trace
payloads). Use it for the `context` field in both Groundedness and
Contradiction.
Added a regression test per check with a context list containing an
apostrophe (the case that most clearly exposes the bug, since Python's repr
mixes quote styles across items to avoid escaping), asserting the rendered
prompt text is exactly the joined documents with no repr artifacts. Updated
the pre-existing tests that had encoded the buggy repr as an assertion.
Verified: new regression tests fail against unmodified judges/*.py (captured
the raw repr and "[]"), pass after the fix. Full giskard-checks suite (759
tests) and every other package (giskard-core, giskard-llm, giskard-agents,
giskard-scan) pass unmodified. ruff format/check clean; basedpyright --level
error (the project's make typecheck) reports 0 errors on touched files.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
@henchaves whenever you get a chance to take a look, happy to address any feedback! |
ebarkhordar
left a comment
There was a problem hiding this comment.
Nice cleanup, the repr leak into judge prompts is a real one.
One asymmetry worth a look: context now routes through format_prompt_text, but answer is still str(...) in get_inputs for both groundedness.py and contradiction.py. When answer is not passed explicitly it is resolved from the trace via answer_key, through the same resolve() in extraction.py, which returns a list whenever the JSONPath is multi-match or a slice/union/wildcard (len(matches) > 1 or _is_list_expression(...)). So an answer_key pointing at a list-valued path would hand the judge "['...', '...']", the same leak this PR fixes for context. The answer: str annotation does not prevent it, since JSONPath resolution is not type-coerced.
Was leaving answer on str() intentional (answer being conceptually single-valued), or worth routing it through format_prompt_text too for symmetry?
GroundednessandContradictionboth acceptcontextasstr | list[str] | MISSING— passing a list of source documents is the documented way to use them (see theGroundednessdocstring example:context=["Paris is the capital of France.", "It's located in Europe."]). The problem was inget_inputs(): whatevercontextresolved to just got passed throughstr(...). For a plain string that's a no-op, but for a list,str()gives you Python'srepr— square brackets, comma separators, and quotes around each item, with the quote character flipping per item whenever one contains an apostrophe (that's just howrepravoids having to escape it). That repr gets dropped straight into the{{ context }}slot ofjudges/groundedness.j2/judges/contradiction.j2, so the judge model was reading something like:instead of the actual context text. An empty list came through as the literal string
"[]".This wasn't some obscure edge case — it was the tested, asserted behavior. Existing tests had lines like
assert result.details["inputs"]["context"] == "['Direct context']", so the repr leak had effectively been locked in as "correct" output.Root cause:
get_inputs()declares the type asstr | list[str]but never actually branches on it — every value falls through to the samestr()call regardless of which half of the union it is.flowchart LR A["context resolves to list[str]<br/>e.g. two context documents, one with an apostrophe"] --> B{"get_inputs()"} B -->|"before: str(value)"| C["Python repr string:<br/>brackets + comma-separated + quoted items<br/>(quote char flips per item)"] B -->|"after: format_prompt_text(value)"| D["items joined with '\\n':<br/>plain readable text, no syntax"] C --> E["rendered into {{ context }} in<br/>groundedness.j2 / contradiction.j2"] D --> E E -->|before| F["judge model sees list/repr syntax<br/>mixed into the prompt"] E -->|after| G["judge model sees the actual<br/>context documents"]Fix is a small
format_prompt_text()helper injudges/base.py: joins list values with\n(matching the"\n".join(...)convention already used elsewhere in this codebase, e.g.llm/translators/google_response.py,checks/export/junit.py), and falls back to plainstr(value)for anything that isn't a list — strings,NoMatch, arbitrary trace payloads, all of which were already rendering correctly. Wired it into thecontextfield of bothGroundedness.get_inputsandContradiction.get_inputs. Went looking for other built-in checks with the samestr | list[str]context pattern; these two are the only ones, so the fix doesn't need to go any further.No issue filed for this — ran into it while going through the judge prompt templates and the rendered context just looked wrong.
Testing
reprmix quote styles. Confirmed both fail against unmodifiedjudges/*.py(actual output was the bracketed repr string), and pass after the fix.test_context_priority_over_tracein both files,test_direct_answer_and_context_are_passed_to_judge,test_custom_answer_and_context_keys).giskard-checkssuite: 759 passed, 4 skipped.giskard-core,giskard-llm,giskard-agents,giskard-scanall green individually — untouched by this change.ruff format --check/ruff checkclean on touched files;basedpyright --level error(whatmake typecheckruns pergiskard-checks/pyproject.toml) reports 0 errors on touched files.Coding agents
I've read AUTONOMOUS.md.
Checklist
CODE_OF_CONDUCT.mddocument.CONTRIBUTING.mdguide.uv.lockrunninguv lock(not applicable — nopyproject.tomlchanges)