Skip to content

Commit a2f25e9

Browse files
test(graphrag): pin why the field-type log is safe, and the bool choice
Review asked whether a non-dict response can reach the new logging line and raise on .items(). It cannot. The two re.sub calls above the parse strip everything outside the outermost braces, so a reply with no braces is reduced to "" and json.loads rejects it before the schema check runs. Anything that survives both regexes starts with "{", so json.loads returns a dict or raises. Pinned with a parametrized case over [], a string array, a bare number and null. Review also asked to reject a boolean rating. Keeping it accepted, and pinning that with a test instead. Nothing reads the rating, so rejecting the value would throw away a usable title, summary and findings, which is the loss this check caused to begin with.
1 parent 6451235 commit a2f25e9

1 file changed

Lines changed: 40 additions & 1 deletion

File tree

test/unit_test/rag/graphrag/test_graphrag_extractors.py

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,13 +94,18 @@ async def slow_async_chat(*_args, **_kwargs):
9494

9595
@staticmethod
9696
def _extract_with_rating(monkeypatch, extractor, rating_literal):
97+
report = '{"title":"Community","summary":"Summary","findings":[],"rating":' + rating_literal + ',"rating_explanation":"Clear"}'
98+
return TestCommunityReportsExtractor._extract_with_response(monkeypatch, extractor, report)
99+
100+
@staticmethod
101+
def _extract_with_response(monkeypatch, extractor, response):
97102
graph = nx.Graph()
98103
graph.add_node("A", description="alpha")
99104
graph.add_node("B", description="beta")
100105
graph.add_edge("A", "B", description="related")
101106

102107
async def fake_async_chat(*_args, **_kwargs):
103-
return '{"title":"Community","summary":"Summary","findings":[],"rating":' + rating_literal + ',"rating_explanation":"Clear"}'
108+
return response
104109

105110
monkeypatch.setattr(
106111
community_reports_module.leiden,
@@ -128,6 +133,40 @@ async def test_report_is_kept_whatever_json_number_the_rating_uses(self, monkeyp
128133
assert len(result.structured_output) == 1
129134
assert result.structured_output[0]["title"] == "Community"
130135

136+
@pytest.mark.p2
137+
@pytest.mark.asyncio
138+
@pytest.mark.parametrize("response", ["[]", '["a","b"]', "5", "null"], ids=["array", "string_array", "number", "null"])
139+
async def test_json_that_is_not_an_object_never_reaches_the_schema_check(self, monkeypatch, response):
140+
"""The two re.sub calls above strip everything outside the outermost braces.
141+
142+
A reply with no braces is reduced to "" and json.loads rejects it, so the
143+
schema check and the logging below only ever see a dict. Pinning that, because
144+
it is what makes the field-type log safe.
145+
"""
146+
extractor = CommunityReportsExtractor(_build_llm_stub())
147+
graph = self._extract_with_response(monkeypatch, extractor, response)
148+
149+
result = await extractor(graph)
150+
151+
assert result.structured_output == []
152+
153+
@pytest.mark.p2
154+
@pytest.mark.asyncio
155+
async def test_a_boolean_rating_is_accepted_rather_than_costing_the_report(self, monkeypatch):
156+
"""isinstance(True, int) is True, so "rating": true passes the widened check.
157+
158+
Deliberate. Nothing reads the rating, so rejecting the value would discard a
159+
usable title, summary and findings, which is the loss this check caused in the
160+
first place. dict_has_keys_with_types treats bool as int elsewhere too, pinned
161+
by test_graphrag_utils.py.
162+
"""
163+
extractor = CommunityReportsExtractor(_build_llm_stub())
164+
graph = self._extract_with_rating(monkeypatch, extractor, "true")
165+
166+
result = await extractor(graph)
167+
168+
assert len(result.structured_output) == 1
169+
131170
@pytest.mark.p2
132171
@pytest.mark.asyncio
133172
async def test_report_with_a_non_numeric_rating_is_still_rejected(self, monkeypatch):

0 commit comments

Comments
 (0)