fix(review): store non-finite evidence as null and count pending items
canonical_evidence now follows main's non-finite policy (#303): NaN and ±Infinity are written as null instead of raising, so a NaN-stamped graph never turns a review item limited. The System Overview reports the review queue's pending total in needs_review.pending, read in the same snapshot as the Library counts.
This commit is contained in:
+5
-2
@@ -637,7 +637,8 @@ class LibraryOperations:
|
||||
pairs (every pair's positive is eligible). ``images`` counts indexed
|
||||
``generated_images`` rows, not files on disk. An image has no rating
|
||||
of its own (it shows its linked prompt's), so ``rated_images`` counts
|
||||
the indexed images whose linked prompt is rated.
|
||||
the indexed images whose linked prompt is rated. ``review_pending`` is
|
||||
the Needs review queue's pending total.
|
||||
"""
|
||||
with self.library_snapshot() as (conn, _epoch, _ceiling):
|
||||
row = conn.execute(
|
||||
@@ -645,13 +646,15 @@ class LibraryOperations:
|
||||
"(SELECT COUNT(*) FROM prompt_pairs WHERE archived=0),"
|
||||
"(SELECT COUNT(*) FROM generated_images),"
|
||||
"(SELECT COUNT(*) FROM generated_images gi JOIN prompts r "
|
||||
"ON r.id=gi.prompt_id WHERE r.rating IS NOT NULL)"
|
||||
"ON r.id=gi.prompt_id WHERE r.rating IS NOT NULL),"
|
||||
"(SELECT COUNT(*) FROM media_review_items WHERE state='pending')"
|
||||
).fetchone()
|
||||
return {
|
||||
"positive_prompts": row[0],
|
||||
"negative_variants": row[1],
|
||||
"images": row[2],
|
||||
"rated_images": row[3],
|
||||
"review_pending": row[4],
|
||||
}
|
||||
|
||||
def usage_by_category(self) -> list[dict]:
|
||||
|
||||
@@ -38,6 +38,11 @@ from .annotation_schema import TABLES as ANNOTATION_TABLES
|
||||
from .annotation_schema import _catalog as _annotation_catalog
|
||||
from .annotation_schema import annotation_schema_error
|
||||
|
||||
try:
|
||||
from ..utils.json_strict import dumps_strict
|
||||
except ImportError:
|
||||
from utils.json_strict import dumps_strict
|
||||
|
||||
SCHEMA_VERSION = 1
|
||||
METADATA_TABLE = "media_review_metadata"
|
||||
ITEMS_TABLE = "media_review_items"
|
||||
@@ -173,12 +178,21 @@ def canonical_evidence(value: Any) -> str:
|
||||
"""Serialize one evidence object canonically; ``ValueError`` when invalid.
|
||||
|
||||
The text is a JSON object with sorted keys, no whitespace, ASCII only and
|
||||
no NaN/infinity, at most ``MAX_EVIDENCE_BYTES`` bytes.
|
||||
no NaN/infinity, at most ``MAX_EVIDENCE_BYTES`` bytes. A non-finite number
|
||||
(ComfyUI stamps ``NaN`` into graphs) is written as ``null``, the store's
|
||||
policy everywhere (``utils.json_strict.dumps_strict``).
|
||||
"""
|
||||
if not isinstance(value, dict):
|
||||
raise ValueError("Review evidence must be a JSON object")
|
||||
try:
|
||||
text = _ENCODER.encode(value)
|
||||
try:
|
||||
text = _ENCODER.encode(value)
|
||||
except ValueError as error:
|
||||
if "Out of range float" not in str(error):
|
||||
raise
|
||||
text = dumps_strict(
|
||||
value, sort_keys=True, separators=(",", ":"), ensure_ascii=True
|
||||
)
|
||||
except (TypeError, RecursionError) as error:
|
||||
raise ValueError(f"Review evidence is not serializable: {error}") from None
|
||||
if len(text) > MAX_EVIDENCE_BYTES:
|
||||
|
||||
+5
-4
@@ -9,9 +9,10 @@
|
||||
kept.
|
||||
- ``GET /prompt_manager/system/overview`` is the System › Overview aggregate.
|
||||
Every value is real or null: there is no backup status until backups report
|
||||
one, no health result until a check ran, and no Needs-review count until
|
||||
the review queue exists. The missing-thumbnail count is not part of it:
|
||||
the page reads ``/images/thumbnails/status``, which walks every source.
|
||||
one and no health result until a check ran; ``needs_review.pending`` is the
|
||||
Needs review queue's pending total. The missing-thumbnail count is not part
|
||||
of it: the page reads ``/images/thumbnails/status``, which walks every
|
||||
source.
|
||||
- ``GET /prompt_manager/system/statistics`` lists runs and ratings per
|
||||
category, with the legacy ``/stats`` totals and the Overview's count of
|
||||
rated images (images whose linked prompt is rated).
|
||||
@@ -174,7 +175,7 @@ class SystemRoutesMixin:
|
||||
},
|
||||
"health": health_summary(self._current_health_report()),
|
||||
"backup": None, # no verified backup status exists yet (S8)
|
||||
"needs_review": {"pending": None}, # no review queue yet (Q6)
|
||||
"needs_review": {"pending": counts["review_pending"]},
|
||||
"sources": {
|
||||
"configured": len(GalleryConfig.MONITORING_DIRECTORIES),
|
||||
"effective": len(output_dirs),
|
||||
|
||||
@@ -536,6 +536,33 @@ class TestDecisions(ReviewRoutesTestCase):
|
||||
self.assertEqual((await self._detail(item["id"]))["state"], "unresolved")
|
||||
self._no_paths()
|
||||
|
||||
async def test_system_overview_counts_pending_items(self):
|
||||
self._require_routes()
|
||||
|
||||
async def pending():
|
||||
status, data = await self._json("GET", "/prompt_manager/system/overview")
|
||||
self.assertEqual(status, 200, data)
|
||||
return data["needs_review"]
|
||||
|
||||
self.assertEqual(await pending(), {"pending": 0})
|
||||
self._record(self._ambiguous("two.png"))
|
||||
self._record(self._ambiguous("other.png", color=(0, 0, 9)))
|
||||
self.assertEqual(await pending(), {"pending": 2})
|
||||
item, detail, _observed = await self._open()
|
||||
body = {
|
||||
"expected_epoch": self.api.db.get_execution_epoch(),
|
||||
"expected_revision": 0,
|
||||
"capture_token": None,
|
||||
"sha256": detail["sha256"],
|
||||
"dependency_fingerprint": detail["dependency_fingerprint"],
|
||||
"choice": "unresolved",
|
||||
}
|
||||
status, data = await self._decide(item["id"], body)
|
||||
self.assertEqual(status, 200, data)
|
||||
# A decided item leaves the count: it is the queue's own total.
|
||||
self.assertEqual(await pending(), {"pending": data["total"]})
|
||||
self.assertEqual(data["total"], 1)
|
||||
|
||||
async def test_changed_bytes_or_evidence_reject_apply(self):
|
||||
self._require_routes()
|
||||
path = self._ambiguous("two.png")
|
||||
|
||||
@@ -1744,6 +1744,7 @@ class LibraryCounts(unittest.TestCase):
|
||||
"negative_variants": 0,
|
||||
"images": 0,
|
||||
"rated_images": 0,
|
||||
"review_pending": 0,
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
@@ -85,6 +85,10 @@ def write_png(path, *, prompt=None, parameters=None, color=(255, 0, 0)):
|
||||
Image.new("RGB", (2, 2), color).save(str(path), pnginfo=info)
|
||||
|
||||
|
||||
def _reject_constant(name):
|
||||
raise ValueError(f"non-strict JSON constant {name}")
|
||||
|
||||
|
||||
def digest(path):
|
||||
return hashlib.sha256(Path(path).read_bytes()).hexdigest()
|
||||
|
||||
@@ -432,6 +436,55 @@ class TestScanRecording(ReviewTestCase):
|
||||
self.assertEqual(len(self._items()), 1)
|
||||
|
||||
|
||||
class TestNonFiniteGraphs(ReviewTestCase):
|
||||
"""ComfyUI stamps NaN/Infinity into graphs; review stores them as null."""
|
||||
|
||||
@staticmethod
|
||||
def _nan_stamped(*positives):
|
||||
graph = ambiguous_graph(*positives)
|
||||
for node in graph.values():
|
||||
node["is_changed"] = [float("nan")]
|
||||
if node["class_type"] == "KSampler":
|
||||
node["inputs"].update({"cfg": float("inf"), "denoise": float("-inf")})
|
||||
return graph
|
||||
|
||||
async def test_scan_records_a_nan_stamped_ambiguous_graph(self):
|
||||
self._require_module()
|
||||
self._roots()
|
||||
path = self.output_dir / "nan.png"
|
||||
# json.dumps' default writes the bare NaN/Infinity literals, as ComfyUI.
|
||||
write_png(path, prompt=self._nan_stamped("cat NaN", "dog Infinity"))
|
||||
self.assertIn(b"NaN", path.read_bytes())
|
||||
|
||||
job = await self._scan()
|
||||
|
||||
self.assertEqual(job["scope"]["review_items"], 1, job)
|
||||
(item,) = self._items()
|
||||
self.assertEqual((item["limited"], item["candidate_count"]), (0, 2))
|
||||
evidence = json.loads(item["evidence"], parse_constant=_reject_constant)
|
||||
self.assertEqual(canonical_evidence(evidence), item["evidence"])
|
||||
self.assertEqual(
|
||||
sorted(c["positive"]["text"] for c in evidence["candidates"]),
|
||||
["cat NaN", "dog Infinity"],
|
||||
)
|
||||
|
||||
async def test_record_review_file_records_a_nan_stamped_graph(self):
|
||||
self._require_module()
|
||||
self._roots()
|
||||
path = self.output_dir / "nan.png"
|
||||
write_png(path, prompt=self._nan_stamped("cat", "dog"))
|
||||
|
||||
self.assertEqual(self.api.record_review_file(str(path)), "recorded")
|
||||
(item,) = self._items()
|
||||
self.assertEqual(item["candidate_count"], 2)
|
||||
|
||||
def test_canonical_evidence_writes_non_finite_numbers_as_null(self):
|
||||
value = {"b": [float("nan"), float("inf")], "a": -float("inf"), "t": "NaN"}
|
||||
text = canonical_evidence(value)
|
||||
self.assertEqual(text, '{"a":null,"b":[null,null],"t":"NaN"}')
|
||||
self.assertEqual(canonical_evidence(json.loads(text)), text)
|
||||
|
||||
|
||||
class TestRecordAmbiguousFile(ReviewTestCase):
|
||||
def _record(self, relative):
|
||||
return review_capture.record_ambiguous_file(
|
||||
|
||||
@@ -644,12 +644,16 @@ class ReviewSchemaTests(unittest.TestCase):
|
||||
self.assertEqual(
|
||||
evidence_digest(EVIDENCE), hashlib.sha256(EVIDENCE.encode()).hexdigest()
|
||||
)
|
||||
# The store's non-finite policy (utils.json_strict): written as null.
|
||||
self.assertEqual(
|
||||
canonical_evidence({"x": float("nan"), "y": [float("-inf")]}),
|
||||
'{"x":null,"y":[null]}',
|
||||
)
|
||||
for bad in (
|
||||
[],
|
||||
"text",
|
||||
{"x": float("nan")},
|
||||
{"x": float("inf")},
|
||||
{"x": object()},
|
||||
{"x": object(), "y": float("nan")},
|
||||
{1: 1, "a": 2},
|
||||
):
|
||||
with self.subTest(value=repr(bad)), self.assertRaises(ValueError):
|
||||
|
||||
@@ -292,7 +292,7 @@ class TestOverviewRoute(SystemAPITestCase):
|
||||
body = await self.json("GET", "/prompt_manager/system/overview")
|
||||
self.assertIsNone(body["health"])
|
||||
self.assertIsNone(body["backup"])
|
||||
self.assertEqual(body["needs_review"], {"pending": None})
|
||||
self.assertEqual(body["needs_review"], {"pending": 0})
|
||||
self.save("Any")
|
||||
await self.run_health()
|
||||
report = (await self.json("GET", "/prompt_manager/system/health"))["report"]
|
||||
|
||||
Reference in New Issue
Block a user