Skip to content

Commit 2934b18

Browse files
committed
fix(diagnostics): strict advisory_only validation + to_dict serialization
- AdvisoryCheck.from_dict: reject malformed strings ('false', '0') — only accept bool or int 0/1, not truthy strings (CodeRabbit MAJOR) - to_dict: serialize AdvisoryCheck instances to dicts — prevents TypeError on JSON serialization (Sentry MEDIUM) - advisory_checks property: explicit comment on except block (CodeQL) - tests: add string rejection test, AdvisoryCheck serialization test, JSON serializability test 80 tests pass.
1 parent 75851e0 commit 2934b18

2 files changed

Lines changed: 48 additions & 5 deletions

File tree

src/qwed_new/core/diagnostics.py

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,19 @@ def to_dict(self) -> Dict[str, Any]:
121121

122122
@classmethod
123123
def from_dict(cls, data: Dict[str, Any]) -> "AdvisoryCheck":
124+
raw_advisory_only = data.get("advisory_only", True)
125+
if isinstance(raw_advisory_only, bool):
126+
advisory_only = raw_advisory_only
127+
elif isinstance(raw_advisory_only, int) and raw_advisory_only in (0, 1):
128+
advisory_only = bool(raw_advisory_only)
129+
else:
130+
raise ValueError(
131+
"AdvisoryCheck.advisory_only must be a bool or integer 0/1"
132+
)
133+
124134
return cls(
125135
name=data.get("name", ""),
126-
advisory_only=bool(data.get("advisory_only", True)),
136+
advisory_only=advisory_only,
127137
constraint_id=data.get("constraint_id"),
128138
details=data.get("details", {}),
129139
)
@@ -263,21 +273,30 @@ def advisory_checks(self) -> List[AdvisoryCheck]:
263273
try:
264274
result.append(AdvisoryCheck.from_dict(item))
265275
except ValueError:
266-
pass
276+
# Invalid advisory metadata is non-authoritative; skip it.
277+
continue
267278
elif isinstance(item, AdvisoryCheck):
268279
result.append(item)
269280
return result
270281

271282
def to_dict(self) -> Dict[str, Any]:
272283
"""Serialize to dict for API/SDK responses and attestation claims.
273284
274-
Returns a flat dict with all three layers. advisory_checks are
275-
serialized as dicts inside developer_fields.
285+
Returns a flat dict with all three layers. AdvisoryCheck instances
286+
in developer_fields['advisory_checks'] are serialized to dicts.
276287
"""
288+
# Deep-copy developer_fields and serialize any AdvisoryCheck instances
289+
fields = dict(self.developer_fields)
290+
checks = fields.get("advisory_checks")
291+
if isinstance(checks, list):
292+
fields["advisory_checks"] = [
293+
item.to_dict() if isinstance(item, AdvisoryCheck) else item
294+
for item in checks
295+
]
277296
return {
278297
"status": self.status.value,
279298
"agent_message": self.agent_message,
280-
"developer_fields": self.developer_fields,
299+
"developer_fields": fields,
281300
"proof_ref": self.proof_ref,
282301
"is_authoritative": self.is_authoritative,
283302
}

tests/test_diagnostics.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,11 @@ def test_from_dict_rejects_zero_advisory_only(self):
265265
with self.assertRaises(ValueError):
266266
AdvisoryCheck.from_dict({"name": "test", "advisory_only": 0})
267267

268+
def test_from_dict_rejects_string_advisory_only(self):
269+
"""from_dict must reject string 'false' — bool('false') is True in Python."""
270+
with self.assertRaises(ValueError):
271+
AdvisoryCheck.from_dict({"name": "test", "advisory_only": "false"})
272+
268273
def test_advisory_check_to_dict(self):
269274
ac = AdvisoryCheck(
270275
name="vlm_analysis",
@@ -373,6 +378,25 @@ def test_verified_round_trip(self):
373378
self.assertEqual(r2.proof_ref, r.proof_ref)
374379
self.assertEqual(r2.developer_fields, r.developer_fields)
375380

381+
def test_to_dict_serializes_advisory_check_instances(self):
382+
"""to_dict must serialize AdvisoryCheck instances to dicts (Sentry MEDIUM)."""
383+
ac = AdvisoryCheck(name="llm", constraint_id="test", details={"verdict": "yes"})
384+
r = DiagnosticResult.unverifiable("no", {"advisory_checks": [ac]})
385+
d = r.to_dict()
386+
checks = d["developer_fields"]["advisory_checks"]
387+
self.assertEqual(len(checks), 1)
388+
self.assertIsInstance(checks[0], dict)
389+
self.assertEqual(checks[0]["name"], "llm")
390+
391+
def test_to_dict_json_serializable_with_advisory_checks(self):
392+
"""to_dict output must be JSON-serializable even with AdvisoryCheck instances."""
393+
import json
394+
ac = AdvisoryCheck(name="nli", constraint_id="graph.nli", details={"label": "entailment"})
395+
r = DiagnosticResult.unverifiable("no", {"advisory_checks": [ac]})
396+
d = r.to_dict()
397+
payload = json.dumps(d) # must not raise TypeError
398+
self.assertIn("nli", payload)
399+
376400
def test_unverifiable_round_trip(self):
377401
r = DiagnosticResult.unverifiable(
378402
"Cannot verify",

0 commit comments

Comments
 (0)