diff --git a/py/api/images.py b/py/api/images.py index db0affd..4f36e4e 100644 --- a/py/api/images.py +++ b/py/api/images.py @@ -30,10 +30,12 @@ from .inspection_service import ( try: from ...database.library import display_title + from ...utils.card_seed import card_seed, header_params, parsed_positive from ...utils.parallel import map_parallel from ...utils.inspection_response import encode_inspection_response except ImportError: from database.library import display_title + from utils.card_seed import card_seed, header_params, parsed_positive from utils.parallel import map_parallel from utils.inspection_response import encode_inspection_response @@ -53,21 +55,40 @@ def _unavailable_link() -> dict: } -def _verified_title(summary: dict | None, observed: dict) -> str | None: - """The inspector's Library title, withholding unconfirmed saved prompt text. +def _verified_title(summary: dict | None, positive: str | None) -> str | None: + """A linked file's Library title, withholding unconfirmed saved prompt text. - A stored title is user data and always shows. A title taken from the saved - positive's first line shows only when the file's own resolved positive gives - the same title, because a saved association never replaces file evidence. + The one rule for the card and the inspector. A stored title is user data and + always shows. A title taken from the saved positive's first line shows only + when ``positive``, the file's own positive as the shared parser reads it from + the file's records, gives the same title: a saved association never replaces + file evidence. """ if not isinstance(summary, dict) or summary.get("status") != "linked": return None title = summary.get("title") if summary.get("title_source") != "text": return title - side = observed.get("prompts", {}).get("positive") or {} - resolved = side.get("status") == "resolved" - return title if resolved and display_title((), side.get("text")) == title else None + confirmed = positive is not None and display_title((), positive) == title + return title if confirmed else None + + +def _card_link(summary: dict, positive: str | None) -> dict: + """The card's link summary with an unconfirmed first-line title withheld.""" + if summary.get("title_source") != "text": + return summary + return {**summary, "title": _verified_title(summary, positive)} + + +def _inspected_positive(observed: dict) -> str | None: + """The positive the shared parser reads from the inspected file's records. + + Videos give None, as on the card, which reads no video metadata. + """ + inspection = observed.get("inspection") + if inspection is None or observed["file"]["media_type"] == "video": + return None + return parsed_positive(header_params(inspection.metadata)) def _public_path(path): @@ -1769,7 +1790,9 @@ class ImageRoutesMixin: ) except CurrentFileChanged: return None - observed["library"]["title"] = _verified_title(summary, observed) + observed["library"]["title"] = _verified_title( + summary, _inspected_positive(observed) + ) return encode_inspection_response( image={ "root_index": index, @@ -1960,20 +1983,23 @@ class ImageRoutesMixin: else {} ) for image in images: - if linked: - key = str(paths[(image["root_index"], image["path"])]) - image["linked"] = summaries.get(key) or _unavailable_link() + path = paths[(image["root_index"], image["path"])] image["width"] = None image["height"] = None - if image["is_video"]: - continue - try: - with paths[(image["root_index"], image["path"])].open("rb") as stream: - with Image.open(stream) as source: - image["width"], image["height"] = source.size - except (OSError, ValueError, Image.DecompressionBombError): - # A corrupt or disappearing preview must not hide the listing. - continue + params = None + if not image["is_video"]: + try: + with path.open("rb") as stream: + with Image.open(stream) as source: + image["width"], image["height"] = source.size + params = header_params(source.info) + except (OSError, ValueError, Image.DecompressionBombError): + # A corrupt or disappearing preview must not hide the listing. + pass + image["seed"] = card_seed(params) + if linked: + summary = summaries.get(str(path)) or _unavailable_link() + image["linked"] = _card_link(summary, parsed_positive(params)) return images def _format_output_page(self, page, *, scoped=False): diff --git a/tests/js/support/inspection-seed.mjs b/tests/js/support/inspection-seed.mjs new file mode 100644 index 0000000..4655236 --- /dev/null +++ b/tests/js/support/inspection-seed.mjs @@ -0,0 +1,30 @@ +// The seed the workspace inspector shows for encoded inspection responses, through +// its own path: projectInspectionPayload with the server facts and the legacy +// parser fill (comfy-metadata.js, passed explicitly; Node has no window global). +// Used by tests/test_api_workspace_images.py to compare card and inspector seeds. +// stdin: [{name, body}] (body is the exact response JSON text). +// stdout: {name: {status, value}}; status 'error' where the page cannot project it. +import { createRequire } from 'node:module'; +import { fileURLToPath, pathToFileURL } from 'node:url'; +import path from 'node:path'; + +const here = path.dirname(fileURLToPath(import.meta.url)); +const web = path.join(here, '..', '..', '..', 'web', 'js'); +const require = createRequire(import.meta.url); +const parser = require(path.join(web, 'comfy-metadata.js')); +const { projectInspectionPayload } = await import(pathToFileURL(path.join(web, 'workspace', 'inspection-state.mjs')).href); + +const chunks = []; +for await (const chunk of process.stdin) chunks.push(chunk); +const output = {}; +for (const { name, body } of JSON.parse(Buffer.concat(chunks).toString('utf8'))) { + const identity = { kind: 'rooted-file', key: `0:${name}`, root_index: 0, path: name, root_scope: null }; + try { + const record = projectInspectionPayload(JSON.parse(body), identity, null, parser.extractGenerationFacts, false, parser); + output[name] = { status: record.generation.seed.status, value: record.generation.seed.value }; + } catch (error) { + // The page shows no record here, so no seed either. + output[name] = { status: 'error', value: String(error && error.name) }; + } +} +process.stdout.write(JSON.stringify(output)); diff --git a/tests/test_api_workspace_images.py b/tests/test_api_workspace_images.py index a5597f5..599a6e6 100644 --- a/tests/test_api_workspace_images.py +++ b/tests/test_api_workspace_images.py @@ -2,28 +2,47 @@ import json import os +import shutil +import subprocess # nosec B404 import unittest from pathlib import Path from unittest.mock import AsyncMock, patch from PIL import Image, PngImagePlugin +from utils.generation_params import extract_from_records + try: from test_api_images import ImageAPITestCase, make_png from open_handles import assert_closed + from generation_params_cases import CASES, encoder, ksampler except ImportError: from tests.test_api_images import ImageAPITestCase, make_png from tests.open_handles import assert_closed + from tests.generation_params_cases import CASES, encoder, ksampler + +SEED_HARNESS = ( + Path(__file__).resolve().parent / "js" / "support" / "inspection-seed.mjs" +) +NODE_TIMEOUT_SECONDS = 120 def embedded_png(path, positive, negative=""): - """A PNG whose API prompt graph resolves to *positive* and *negative*.""" + """A PNG whose API prompt graph resolves to *positive* and *negative*. + + The sampler has a model link, as every ComfyUI KSampler does, so the shared + parser (which needs one) and the server resolver both read the prompts. + """ graph = { "1": {"class_type": "CLIPTextEncode", "inputs": {"text": positive}}, "2": {"class_type": "CLIPTextEncode", "inputs": {"text": negative}}, "3": { "class_type": "KSampler", - "inputs": {"positive": ["1", 0], "negative": ["2", 0]}, + "inputs": {"positive": ["1", 0], "negative": ["2", 0], "model": ["4", 0]}, + }, + "4": { + "class_type": "CheckpointLoaderSimple", + "inputs": {"ckpt_name": "fixture.safetensors"}, }, } info = PngImagePlugin.PngInfo() @@ -34,6 +53,32 @@ def embedded_png(path, positive, negative=""): return path +def records_png(path, records, mtime=100): + """A PNG carrying the given text records (None values are left out).""" + info = PngImagePlugin.PngInfo() + for key, text in records.items(): + if text is not None: + info.add_text(key, text) + with Image.new("RGB", (4, 4)) as image: + image.save(path, pnginfo=info) + os.utime(path, (mtime, mtime)) + return path + + +def seeded_records(positive, seed): + """API prompt records for a one-sampler graph with *positive* and *seed*.""" + graph = { + "4": { + "class_type": "CheckpointLoaderSimple", + "inputs": {"ckpt_name": "concept.safetensors"}, + }, + "6": encoder(positive), + "7": encoder("blurry"), + "3": ksampler(("6", 0), ("7", 0), seed=seed), + } + return {"prompt": json.dumps(graph)} + + class TestWorkspaceImages(ImageAPITestCase): async def tearDownAsync(self): self.api.db.close_all() @@ -430,7 +475,6 @@ class TestWorkspaceImages(ImageAPITestCase): self.assertEqual(library["title"], linked["title"]) self.assertEqual(library["positive_id"], prompt) - @unittest.expectedFailure # B2 gates text titles on the file's parsed positive. async def test_unconfirmed_text_title_is_withheld_on_card_and_inspector(self): prompt = self._save_prompt("Secret first line\nrest") path = self.media("plain.png") @@ -523,6 +567,136 @@ class TestWorkspaceImages(ImageAPITestCase): self.assertIsNone(data["images"][0]["linked"]["title_source"]) self.assertNotIn("database is gone", str(data)) + async def test_items_carry_seed_from_header_records(self): + records_png(self.output_dir / "seeded.png", seeded_records("a fox", 42001)) + self.media("plain.png") + self.media("clip.mp4") + self.media("broken.png").write_bytes(b"invalid") + _, data = await self.get() + seeds = {item["filename"]: item["seed"] for item in data["images"]} + self.assertEqual( + seeds, + { + "seeded.png": 42001, + "plain.png": None, + "clip.mp4": None, + "broken.png": None, + }, + ) + + async def test_card_reads_each_header_once(self): + for index in range(3): + records_png( + self.output_dir / f"{index}.png", seeded_records("a fox", index) + ) + self.media("clip.mp4") + with patch("PIL.Image.open", wraps=Image.open) as opened: + _, data = await self.get() + self.assertEqual(len(data["images"]), 4) + self.assertEqual(opened.call_count, 3) + + def _seed_files(self): + """Concept-style one-sampler files plus every shared parser case.""" + expected = {} + for i in range(2): + for j in range(3): + name = f"concept-{i}-{j}.png" + seed = 42001 + i * 100 + j + records_png( + self.output_dir / name, seeded_records(f"concept {i} {j}", seed) + ) + expected[name] = seed + # ComfyUI's randomized seeds mostly exceed 2**53, where JSON.parse rounds. + big = 2**63 + 12345 + records_png(self.output_dir / "big-seed.png", seeded_records("big", big)) + expected["big-seed.png"] = big + for case, records in CASES.items(): + records_png(self.output_dir / f"case-{case}.png", records) + records_png( + self.output_dir / "a1111.png", + {"parameters": "a fox\nNegative prompt: blur\nSteps: 20, Seed: 9"}, + ) + self.media("bare.png") + return expected + + async def _inspector_seeds(self, items): + node = shutil.which("node") + if node is None: # pragma: no cover - CI runners and dev hosts ship Node + if os.environ.get("CI"): + self.fail("Node.js is required in CI for the seed parity check") + self.skipTest("Node.js is not installed") + bodies = [] + for item in items: + response = await self.client.get( + f"/prompt_manager/images/inspect/{item['path']}", + params={"root": 0, "root_scope": item["root_scope"]}, + ) + self.assertEqual(response.status, 200, item["path"]) + bodies.append({"name": item["path"], "body": await response.text()}) + # Fixed local Node harness, no shell. + completed = subprocess.run( # nosec B603 + [node, str(SEED_HARNESS)], + input=json.dumps(bodies).encode("utf-8"), + capture_output=True, + timeout=NODE_TIMEOUT_SECONDS, + check=False, + ) + self.assertEqual(completed.returncode, 0, completed.stderr.decode("utf-8")) + return json.loads(completed.stdout) + + async def test_card_seed_equals_inspector_seed_when_both_present(self): + expected = self._seed_files() + _, data = await self.get(limit=200) + items = data["images"] + self.assertEqual(len(items), len(CASES) + len(expected) + 2) + inspector = await self._inspector_seeds(items) + both = 0 + for item in items: + with self.subTest(file=item["path"]): + card, shown = item["seed"], inspector[item["path"]] + if item["path"] in expected: + self.assertEqual(card, expected[item["path"]]) + if card is None: + continue + # The card never shows a seed the inspector does not. + self.assertEqual(shown, {"status": "known", "value": str(card)}) + both += 1 + self.assertGreaterEqual(both, len(expected) + 10) + + async def test_text_title_needs_the_files_parsed_positive(self): + prompt = self._save_prompt("Saved words\nrest") + other = records_png( + self.output_dir / "other.png", seeded_records("Different words", 1) + ) + shared = records_png( + self.output_dir / "shared.png", CASES["shared-prompt-pipeline"] + ) + clip = self.media("clip.mp4") + shared_prompt = self._save_prompt( + extract_from_records(CASES["shared-prompt-pipeline"]["prompt"], None)[ + "positivePrompt" + ] + ) + for path, owner in ((other, prompt), (shared, shared_prompt), (clip, prompt)): + self.api.db.link_image_to_prompt(owner, str(path)) + _, data = await self.get() + items = {item["filename"]: item for item in data["images"]} + for name in ("other.png", "clip.mp4"): + with self.subTest(file=name): + self.assertEqual(items[name]["linked"]["status"], "linked") + self.assertEqual(items[name]["linked"]["title_source"], "text") + self.assertIsNone(items[name]["linked"]["title"]) + library = await self.inspected("other.png", items["other.png"]) + self.assertIsNone(library["title"]) + self.assertNotIn("Saved words", json.dumps(data)) + # The resolver cannot decide this graph; the shared parser (and so the + # inspector's prompt) confirms the saved first line on both. + title = items["shared.png"]["linked"]["title"] + self.assertIsNotNone(title) + self.assertEqual( + (await self.inspected("shared.png", items["shared.png"]))["title"], title + ) + class TestWorkspaceCursorEncoding(unittest.TestCase): def test_ntfs_unicode_basename_cursor_is_bounded_and_round_trips(self): diff --git a/tests/test_card_seed.py b/tests/test_card_seed.py new file mode 100644 index 0000000..b0730a1 --- /dev/null +++ b/tests/test_card_seed.py @@ -0,0 +1,170 @@ +"""Card facts from header records: the shared legacy parser, bounded and exact.""" + +import json +import os +import sys +import unittest +from unittest.mock import patch + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +from utils.card_seed import ( # noqa: E402 + MAX_RECORD_BYTES, + card_seed, + header_params, + parsed_positive, +) +from utils.generation_params import ( # noqa: E402 + GenerationParamsError, + extract_from_records, +) + +try: + from tests.generation_params_cases import CASES, encoder, ksampler # noqa: E402 +except ImportError: # discovered with tests/ as the top-level directory + from generation_params_cases import CASES, encoder, ksampler # noqa: E402 + + +def graph(*samplers, extra=None): + nodes = {"6": encoder("a lantern at dusk"), "7": encoder("blurry")} + for index, sampler in enumerate(samplers): + nodes[str(10 + index)] = sampler + nodes.update(extra or {}) + return {"prompt": json.dumps(nodes)} + + +def seeded(seed, **settings): + return ksampler(("6", 0), ("7", 0), seed=seed, **settings) + + +def info_seed(info): + return card_seed(header_params(info)) + + +class CardSeed(unittest.TestCase): + def test_card_seed_single_sampler(self): + info = graph(seeded(42001)) + self.assertEqual(info_seed(info), 42001) + self.assertEqual(parsed_positive(header_params(info)), "a lantern at dusk") + + def test_card_seed_two_different_seeds_follows_shared_parser(self): + # Ruling: no separate heuristic; the card shows what the legacy parser + # (and so the inspector) shows for several samplers. + info = graph(seeded(111), seeded(222)) + expected = extract_from_records(info["prompt"], None)["seed"] + self.assertIn(expected, (111, 222)) + self.assertEqual(info_seed(info), expected) + + def test_card_seed_linked_input_follows_shared_parser(self): + primitive = {"class_type": "PrimitiveNode", "inputs": {"value": 555}} + info = graph(seeded(["20", 0]), extra={"20": primitive}) + self.assertEqual(info_seed(info), 555) + dangling = graph(seeded(["99", 0])) + self.assertEqual( + extract_from_records(dangling["prompt"], None)["seed"], "Unknown" + ) + self.assertIsNone(info_seed(dangling)) + + def test_card_seed_advanced_sampler_noise_seed(self): + advanced = seeded(None) + advanced["class_type"] = "KSamplerAdvanced" + del advanced["inputs"]["seed"] + advanced["inputs"]["noise_seed"] = 77 + self.assertEqual(info_seed(graph(advanced)), 77) + + def test_card_seed_oversized_record_is_none(self): + padding = "x" * (MAX_RECORD_BYTES + 1) + big_prompt = graph(seeded(1), extra={"30": encoder(padding)}) + self.assertGreater(len(big_prompt["prompt"]), MAX_RECORD_BYTES) + self.assertIsNone(header_params(big_prompt)) + big_workflow = {**graph(seeded(1)), "workflow": json.dumps({"x": padding})} + self.assertIsNone(header_params(big_workflow)) + # Bytes, not characters: two-byte text over the budget is oversized. + wide = "é" * (MAX_RECORD_BYTES // 2 + 1) + self.assertLessEqual(len(wide), MAX_RECORD_BYTES) + self.assertIsNone(header_params({"prompt": json.dumps({"a": wide})})) + self.assertEqual(info_seed(graph(seeded(1))), 1) + + def test_placeholder_and_non_number_seeds_are_none(self): + for value in ("123", "randomize", True, 1.5, None): + with self.subTest(seed=value): + self.assertIsNone(info_seed(graph(seeded(value)))) + self.assertIsNone(card_seed({"seed": "Unknown"})) + self.assertIsNone(card_seed({"seed": float("inf")})) + self.assertIsNone(card_seed({"seed": 1e21})) + self.assertEqual(card_seed({"seed": -3}), -3) + self.assertEqual(card_seed({"seed": 7.0}), 7) + + def test_seed_beyond_double_precision_is_the_recorded_integer(self): + # JSON.parse rounds it; the inspector shows the record's own digits. + case = CASES["seed-beyond-double-precision"] + rounded = extract_from_records(case["prompt"], None)["seed"] + self.assertEqual(rounded, float(4974052037430000001)) + self.assertEqual(info_seed({"prompt": case["prompt"]}), 4974052037430000001) + + def test_big_seed_stays_rounded_when_the_exact_parse_disagrees(self): + # Only a seed that rounds to the parsed value replaces it; otherwise the + # card prints the double as the browser does (String(2**60)). + browser = 1152921504606847000 + self.assertEqual(card_seed({"seed": float(2**60)}), browser) + with patch( + "utils.card_seed.extract_generation_params", + return_value={"seed": 2**61}, + ): + params = header_params(graph(seeded(2**60 + 1))) + self.assertEqual(card_seed(params), browser) + with patch( + "utils.card_seed.extract_generation_params", + side_effect=GenerationParamsError("throws"), + ): + params = header_params(graph(seeded(2**60 + 1))) + self.assertEqual(card_seed(params), browser) + self.assertEqual(info_seed(graph(seeded(-(2**60) - 1))), -(2**60) - 1) + + def test_workflow_only_record(self): + case = next( + case + for case in CASES.values() + if case["prompt"] is None + and case["workflow"] is not None + and isinstance(extract_from_records(None, case["workflow"])["seed"], int) + ) + expected = extract_from_records(None, case["workflow"])["seed"] + self.assertEqual(info_seed({"workflow": case["workflow"]}), expected) + + def test_records_the_parser_cannot_use_give_nothing(self): + self.assertIsNone(header_params({})) + self.assertIsNone(header_params(None)) + self.assertIsNone(header_params({"parameters": "a cat\nSteps: 20, Seed: 9"})) + self.assertIsNone(header_params({"prompt": b"{}"})) + self.assertIsNone(card_seed(None)) + self.assertIsNone(parsed_positive(None)) + # The browser parser throws here; the card shows nothing. + thrown = CASES["null-workflow-node"] + self.assertIsNone(header_params(thrown)) + unparsable = header_params(CASES["unparsable-records"]) + self.assertIsNone(card_seed(unparsable)) + self.assertIsNone(parsed_positive(unparsable)) + + def test_every_shared_case_matches_the_parser(self): + for name, case in CASES.items(): + with self.subTest(case=name): + info = {k: v for k, v in case.items() if v is not None} + params = header_params(info) + if params is None: + continue + parsed = extract_from_records(case["prompt"], case["workflow"]) + if name == "seed-beyond-double-precision": + parsed = {**parsed, "seed": 4974052037430000001} + self.assertEqual(params, parsed) + positive = params["positivePrompt"] + usable = ( + isinstance(positive, str) + and positive != "No prompt found" + and positive.strip() + ) + self.assertEqual(parsed_positive(params), positive if usable else None) + + +if __name__ == "__main__": + unittest.main() diff --git a/utils/card_seed.py b/utils/card_seed.py new file mode 100644 index 0000000..a2fe2b0 --- /dev/null +++ b/utils/card_seed.py @@ -0,0 +1,127 @@ +"""Workspace card facts from the metadata records a header open already loaded. + +The card shows what the inspector shows, so it reads the same shared parser +(``extract_generation_params``, the Python twin of the browser parser the +inspector fills from since #251) and nothing else: no separate seed heuristic, +no graph resolution and no extra file reads. One parse feeds both the seed and +the positive that confirms a first-line card title. +""" + +import json +import math +import re +from collections.abc import Mapping + +from .generation_params import ( + PLACEHOLDERS, + GenerationParamsError, + _number_string, + extract_from_records, + extract_generation_params, +) + +__all__ = ["MAX_RECORD_BYTES", "card_seed", "header_params", "parsed_positive"] + +# Each record is parsed only up to this many UTF-8 bytes; past it, nothing is read. +MAX_RECORD_BYTES = 2 * 1024 * 1024 +_RECORD_KEYS = ("prompt", "workflow") +_INTEGER = re.compile(r"-?[0-9]+\Z") +_MAX_SAFE_INTEGER = 2**53 - 1 + + +def header_params(info) -> dict | None: + """The shared parser's result for the ``prompt``/``workflow`` text in ``info``. + + ``info`` is a Pillow ``Image.info`` (or the inspection read's metadata). + None when neither record is present, a record is not text or is over + ``MAX_RECORD_BYTES``, or the browser parser would throw on it. + """ + if not isinstance(info, Mapping): + return None + records = {key: info.get(key) for key in _RECORD_KEYS} + if all(text is None for text in records.values()): + return None + for text in records.values(): + if text is None: + continue + if not isinstance(text, str): + return None + if len(text.encode("utf-8", "surrogatepass")) > MAX_RECORD_BYTES: + return None + try: + params = extract_from_records(records["prompt"], records["workflow"]) + except GenerationParamsError: + return None + return _with_exact_seed(params, records) + + +def _reject_constant(name): + raise ValueError(f"{name} is not JSON") + + +def _exact_json(text): + """``text`` parsed with integers kept exact, or None where JSON.parse throws.""" + if not isinstance(text, str): + return None + try: + return json.loads(text, parse_constant=_reject_constant) + except (ValueError, RecursionError): + return None + + +def _with_exact_seed(params: dict, records: dict) -> dict: + """``params`` with a rounded seed replaced by the integer the file records. + + ``JSON.parse`` rounds integers beyond 2**53, but the inspector reads the + record's own digits (its projection carries big numbers as text). ComfyUI's + randomized seeds are mostly that large, so the same parser is run once more + on an exact parse of the same records, and its seed is kept only when it is + the one that rounded to the parsed value. Anything else leaves the seed + as parsed. + """ + seed = params.get("seed") + if not isinstance(seed, float) or not math.isfinite(seed): + return params + if abs(seed) <= _MAX_SAFE_INTEGER or not seed.is_integer(): + return params + try: + exact = extract_generation_params( + { + key: _exact_json(text) + for key, text in records.items() + if text is not None + } + )["seed"] + except GenerationParamsError: + return params + if isinstance(exact, bool) or not isinstance(exact, int) or float(exact) != seed: + return params + return {**params, "seed": exact} + + +def card_seed(params: dict | None) -> int | None: + """The parsed seed as the integer the browser prints, else None. + + A placeholder, text, boolean or non-integral number gives None. A double + that ``header_params`` could not make exact gives the integer whose decimal + form equals the browser's ``String(seed)``. + """ + if not isinstance(params, Mapping): + return None + value = params.get("seed") + if isinstance(value, bool) or not isinstance(value, (int, float)): + return None + if isinstance(value, float) and not math.isfinite(value): + return None + text = _number_string(value) + return int(text) if _INTEGER.match(text) else None + + +def parsed_positive(params: dict | None) -> str | None: + """The parsed positive prompt text, or None for the placeholder or blank.""" + if not isinstance(params, Mapping): + return None + text = params.get("positivePrompt") + if not isinstance(text, str) or text == PLACEHOLDERS["positivePrompt"]: + return None + return text if text.strip() else None