diff --git a/TESTING.md b/TESTING.md index 6b44b5e3..c424ff90 100644 --- a/TESTING.md +++ b/TESTING.md @@ -801,13 +801,16 @@ during that run (the #342 rules): the one output node that run reported it from when several did) and, per sampler anchor, the effective texts PromptManager recorded at execution (a role only inferred from the graph is none). Only when the file alone leaves the prompt open (`ambiguous` or `unresolved`) the inspector resolves again -anchored on that node (`recorded_saver_id`, walked like a file-name match), and keeps -that result only when it names the positive (resolved, or a guider with no -negative); its `graph-inferred` sides read `recorded-run` ("Recorded run" badge). -Still open at one named sampler whose positive text was recorded (a language -model's output feeding PromptManager), the recorded texts show as `execution`. A -result the file resolves alone never changes; a lookup failure is logged and judges -the file alone; local Inspect and ingestion are unchanged +anchored on that node (`recorded_saver_id`, walked like a file-name match; a file +name naming a different saver contradicts history and anchors nothing), and keeps +that result only when history's saver chose its samplers (`recorded_anchor`) and it +names the positive (resolved, or a guider with no negative); its `graph-inferred` +sides read `recorded-run` ("Recorded run" badge, ranked below every "Workflow +inferred" source). Still open at the one sampler that saver leads to, with that +sampler's positive text recorded (a language model's output feeding PromptManager), +the recorded texts show as `execution`. Outputs and texts are read in one +transaction. A result the file resolves alone never changes; a lookup failure is +logged and judges the file alone; local Inspect and ingestion are unchanged (`tests/test_recorded_saver_anchor.py`, `tests/test_run_attribution.py`, `tests/test_api_history_outputs.py`). diff --git a/database/run_attribution.py b/database/run_attribution.py index fb969574..2f0cbd99 100644 --- a/database/run_attribution.py +++ b/database/run_attribution.py @@ -281,23 +281,20 @@ def recorded_output( """ if not image_path: return None - found = _recorded_runs(db, [image_path], output_root, path_style) - if image_path not in found: - return None - run_id, nodes = found[image_path] - saver = next(iter(nodes)) if len(nodes) == 1 else None - return RecordedOutput(saver, _recorded_texts(db, run_id)) - - -def recorded_output_node( - db: Any, - image_path: str, - output_root: Optional[str], - path_style: Optional[str] = None, -) -> Optional[str]: - """The one output node history says wrote ``image_path``, else None.""" - found = recorded_output(db, image_path, output_root, path_style) - return found.saver_id if found is not None else None + conn = db.model.get_connection() + own_transaction = not conn.in_transaction + if own_transaction: # outputs and texts from one state of history + conn.execute("BEGIN") + try: + found = _recorded_runs(db, [image_path], output_root, path_style) + if image_path not in found: + return None + run_id, nodes = found[image_path] + saver = next(iter(nodes)) if len(nodes) == 1 else None + return RecordedOutput(saver, _recorded_texts(db, run_id)) + finally: + if own_transaction: + conn.rollback() _LOGGER = logging.getLogger("prompt_manager.run_attribution") @@ -315,9 +312,12 @@ def recorded_output_lookup(db: Any, *, close: bool = False) -> Callable[[Any], A def lookup(path: Any) -> Optional[RecordedOutput]: try: return recorded_output(db, str(path), host_output_directory()) - except Exception: + except (sqlite3.Error, OSError, ValueError): _LOGGER.warning("Recorded run lookup failed", exc_info=True) return None + except Exception: + _LOGGER.exception("Recorded run lookup failed unexpectedly") + return None finally: if close: db.model.close() diff --git a/tests/js/workspace-file-inspector.test.js b/tests/js/workspace-file-inspector.test.js index 8068c618..11d122ca 100644 --- a/tests/js/workspace-file-inspector.test.js +++ b/tests/js/workspace-file-inspector.test.js @@ -198,6 +198,11 @@ test('a branch named by recorded run history shows the Recorded run badge', asyn try { assert.deepEqual(view.texts(n => n.className === 'chip blue'), ['Execution recorded']); } finally { view.done(); } + // A recorded branch whose negative left out a run-time part is still inferred from the workflow. + view = renderFull({...recorded, prompts:{...recorded.prompts, negative:{...recorded.prompts.negative, source:'runtime-parts-omitted'}}}); + try { + assert.deepEqual(view.texts(n => n.className === 'chip blue'), ['Workflow inferred']); + } finally { view.done(); } }); // #249 PR C Task C3 (D88): one chip row, the file's current image tags first, then D21's linked tags not already shown. diff --git a/tests/test_recorded_saver_anchor.py b/tests/test_recorded_saver_anchor.py index c606f561..3ca852c6 100644 --- a/tests/test_recorded_saver_anchor.py +++ b/tests/test_recorded_saver_anchor.py @@ -158,6 +158,24 @@ class TestRecordedSaverInInspector(unittest.TestCase): ) self.assertEqual(evidence["prompts"]["negative"]["status"], "absent") + def test_a_file_name_naming_another_saver_contradicts_history(self): + # The bench file name names branch 2's saver; history says branch 4's. + graph = shapes.branch_bench() + for index in (1, 3, 4): + graph[_saver(index)]["inputs"]["filename_prefix"] = "bench/run/shared" + result = _resolve( + graph, file_name="s45_branch_2_00001_.png", recorded_saver_id=_saver(4) + ) + self.assertIs(result["recorded_anchor"], False) + evidence = _evidence( + shapes.shared_prefix(), + file_name="IMG_GEN_00001_.png", + recorded_saver_id=_saver(4), + ) + self.assertEqual( + evidence["prompts"]["positive"]["text"], shapes.BRANCH_TEXTS[3] + ) + def test_a_file_that_resolves_alone_keeps_its_own_result(self): # The file name names branch 2; a recorded saver never overrides it. evidence = _evidence( @@ -240,6 +258,35 @@ def text_generate_run(): } +def two_generated_branches(): + """``text_generate_run`` plus a second generated branch saved by an unknown + saver and also previewed.""" + graph = text_generate_run() + graph["31"] = {**graph["11"], "inputs": {**graph["11"]["inputs"]}} + graph["37"] = { + "class_type": "PromptManagerPair", + "inputs": {**graph["36"]["inputs"], "text": ["31", 0]}, + } + graph["20"] = { + "class_type": "KSampler", + "inputs": { + **graph["7"]["inputs"], + "positive": ["37", 0], + "negative": ["37", 1], + }, + } + graph["21"] = { + "class_type": "VAEDecode", + "inputs": {"samples": ["20", 0], "vae": ["1", 2]}, + } + graph["22"] = { + "class_type": "Image Save", + "inputs": {"filename_prefix": "other", "images": ["21", 0]}, + } + graph["23"] = {"class_type": "PreviewImage", "inputs": {"images": ["21", 0]}} + return graph + + class TestRecordedTexts(unittest.TestCase): """The texts PromptManager recorded at execution, per sampler of the run.""" @@ -269,6 +316,7 @@ class TestRecordedTexts(unittest.TestCase): def test_an_unrecorded_negative_stays_open(self): evidence = _evidence( text_generate_run(), + recorded_saver_id="9", recorded_texts={"7": {"positive": LLM_TEXT, "negative": None}}, ) self.assertEqual(evidence["prompts"]["positive"]["text"], LLM_TEXT) @@ -285,24 +333,46 @@ class TestRecordedTexts(unittest.TestCase): ): with self.subTest(texts=texts): self.assertEqual( - _evidence(text_generate_run(), recorded_texts=texts), alone + _evidence( + text_generate_run(), recorded_saver_id="9", recorded_texts=texts + ), + alone, ) - def test_recorded_texts_never_replace_a_resolved_or_several_outcome(self): - texts = {"21": {"positive": LLM_TEXT, "negative": ""}} - resolved = _evidence(shapes.branch_bench(), file_name="s45_branch_2_00001_.png") - self.assertEqual( - _evidence( - shapes.branch_bench(), - file_name="s45_branch_2_00001_.png", - recorded_texts=texts, - ), - resolved, - ) - several = _evidence(shapes.branch_bench()) - self.assertEqual( - _evidence(shapes.branch_bench(), recorded_texts=texts), several - ) + def test_texts_need_the_saver_history_recorded_to_lead_to_the_sampler(self): + texts = {"7": {"positive": LLM_TEXT, "negative": ""}} + alone = _evidence(text_generate_run()) + for saver in (None, "missing", "2"): + with self.subTest(saver=saver): + self.assertEqual( + _evidence( + text_generate_run(), + recorded_saver_id=saver, + recorded_texts=texts, + ), + alone, + ) + + def test_texts_follow_the_recorded_saver_not_another_sampler(self): + # Review: the texts are those of the sampler history's saver leads to. + graph = two_generated_branches() + texts = { + "7": {"positive": "STAGE X TEXT", "negative": ""}, + "20": {"positive": "STAGE Y TEXT", "negative": ""}, + } + for saver, text, sampler in ( + ("22", "STAGE Y TEXT", "20"), + ("9", "STAGE X TEXT", "7"), + ): + with self.subTest(saver=saver): + evidence = _evidence( + graph, + file_name="renamed_00001_.png", + recorded_saver_id=saver, + recorded_texts=texts, + ) + self.assertEqual(evidence["prompts"]["positive"]["text"], text) + self.assertEqual(evidence["resolution"]["sampler_id"], sampler) if __name__ == "__main__": diff --git a/tests/test_run_attribution.py b/tests/test_run_attribution.py index 32f925ca..592a7768 100644 --- a/tests/test_run_attribution.py +++ b/tests/test_run_attribution.py @@ -12,7 +12,6 @@ from unittest import mock from database.run_attribution import ( RecordedOutput, recorded_output, - recorded_output_node, stored_run_prompt, ) from tests import test_execution_history_database as history_tests @@ -195,7 +194,8 @@ class TestRecordedOutputNode(RecordedRuns, unittest.TestCase): setUp = TestStoredRunPrompt.setUp def node(self, path=None): - return recorded_output_node(self.db, path or self.image, self.output) + found = recorded_output(self.db, path or self.image, self.output) + return found.saver_id if found is not None else None def test_one_run_one_saver_names_the_saver(self): other = os.path.join(self.output, "sub", "img_00002_.png") @@ -248,8 +248,8 @@ class TestRecordedOutputNode(RecordedRuns, unittest.TestCase): other = os.path.join(self.output, "sub", "other.png") Path(other).write_bytes(b"png") self.assertIsNone(self.node(other)) - self.assertIsNone(recorded_output_node(self.db, self.image, None)) - self.assertIsNone(recorded_output_node(self.db, "", self.output)) + self.assertIsNone(recorded_output(self.db, self.image, None)) + self.assertIsNone(recorded_output(self.db, "", self.output)) def test_the_run_texts_recorded_at_execution_come_with_the_saver(self): self.record_run( @@ -341,7 +341,7 @@ class TestRecordedOutputNode(RecordedRuns, unittest.TestCase): broken.model.get_connection.return_value.execute.side_effect = ( sqlite3.OperationalError("no such table") ) - self.assertIsNone(recorded_output_node(broken, self.image, self.output)) + self.assertIsNone(recorded_output(broken, self.image, self.output)) class TestScanUsesHistory(RecordedRuns, AdminAPITestCase): diff --git a/utils/prompt_inspector.py b/utils/prompt_inspector.py index 1e0ed6ed..1daaef6c 100644 --- a/utils/prompt_inspector.py +++ b/utils/prompt_inspector.py @@ -661,14 +661,16 @@ def inspect_prompt_evidence( file_name=file_name, recorded_saver_id=recorded_saver_id, ) - if anchored["state"] not in _UNDECIDED and isinstance( - anchored["positive"], dict - ): - resolved, recorded = anchored, True - if resolved["state"] in _UNDECIDED and recorded_texts: - executed = _recorded_execution(resolved, recorded_texts) - if executed is not None: - return executed + # Only a result whose samplers history's saver chose is about this file. + if anchored.get("recorded_anchor") is True: + if anchored["state"] not in _UNDECIDED and isinstance( + anchored["positive"], dict + ): + resolved, recorded = anchored, True + elif recorded_texts: + executed = _recorded_execution(anchored, recorded_texts) + if executed is not None: + return executed state = resolved["state"] result = {} for role in ("positive", "negative"): diff --git a/utils/prompt_pair_resolver.py b/utils/prompt_pair_resolver.py index 1080f0b2..f22f1164 100644 --- a/utils/prompt_pair_resolver.py +++ b/utils/prompt_pair_resolver.py @@ -1601,6 +1601,11 @@ class _GraphResolver: several, an unreadable walk or no candidate sampler is ``None``: the file is then judged unanchored, never guessed (#381). """ + match = self.saver_match(file_name) + return self.saver_walk(match) if match is not None else None + + def saver_match(self, file_name: Any) -> Optional[str]: + """The one node whose ``filename_prefix`` names ``file_name``, or None.""" stem = _file_stem(file_name) if stem is None: return None @@ -1613,9 +1618,7 @@ class _GraphResolver: ] except (_Unresolved, _Diverging): return None - if len(matches) != 1: - return None - return self.saver_walk(matches[0]) + return matches[0] if len(matches) == 1 else None def recorded_saver_stages(self, node_id: str) -> Optional[list]: """The candidate samplers of the save node recorded history names (#386). @@ -2404,10 +2407,11 @@ def _resolve( "unresolved", reason=str(error), reason_code=error.code, node=error.node ) else: - anchored = None + anchored, by_record = None, False if semantics is not None and recorded_saver_id is not None: - anchored = resolver.recorded_saver_stages(str(recorded_saver_id)) - if anchored is None and semantics is not None and file_name is not None: + anchored = _recorded_anchor(resolver, str(recorded_saver_id), file_name) + by_record = anchored is not None + elif semantics is not None and file_name is not None: anchored = resolver.saver_stages(file_name) sampler_ids = anchored or [ node_id @@ -2418,6 +2422,34 @@ def _resolve( ] if semantics is not None and anchored is None: sampler_ids = resolver.final_stages(sampler_ids) + if semantics is not None and recorded_saver_id is not None: + # The caller can tell whether history's saver chose the samplers. + return { + **_judge(resolver, sampler_ids, normalized, profile), + "recorded_anchor": by_record, + } + return _judge(resolver, sampler_ids, normalized, profile) + + +def _recorded_anchor( + resolver: "_GraphResolver", saver_id: str, file_name: Optional[str] +) -> Optional[list]: + """The samplers of the saver history recorded, or None (#386). + + A file name that names a different saver contradicts history: no anchor. + """ + named = resolver.saver_match(file_name) if file_name is not None else None + if named is not None and named != saver_id: + return None + return resolver.recorded_saver_stages(saver_id) + + +def _judge( + resolver: "_GraphResolver", + sampler_ids: list, + normalized: dict, + profile: ResolverProfile, +) -> Dict[str, Any]: if not sampler_ids: return _no_sampler(normalized, profile) if len(sampler_ids) > 1: diff --git a/web/js/workspace/file-inspector.mjs b/web/js/workspace/file-inspector.mjs index 3cdc7461..c6292363 100644 --- a/web/js/workspace/file-inspector.mjs +++ b/web/js/workspace/file-inspector.mjs @@ -158,9 +158,11 @@ function generationFacts(record, listClass, noteClass) { } // D21: the panel badge names the strongest resolved prompt source; any other source shows no badge. // A prompt that left out a run-time PromptManager part (D125) or a later stage's added conditioning (D126) is still -// inferred from the workflow. "Recorded run" (#386): recorded run history named the save node that wrote the file. -const EVIDENCE_BADGES = [['execution', 'Execution recorded'], ['recorded-run', 'Recorded run'], ['graph-inferred', 'Workflow inferred'], ['runtime-parts-omitted', 'Workflow inferred'], - ['added-conditioning', 'Workflow inferred'], ['runtime-parts-omitted-and-added-conditioning', 'Workflow inferred'], ['embedded-parameters', 'Embedded parameters']]; +// inferred from the workflow. "Recorded run" (#386): recorded run history named the save node that wrote the file; it +// ranks below any workflow-inferred side, so a branch with an omitted or added part still reads "Workflow inferred". +const EVIDENCE_BADGES = [['execution', 'Execution recorded'], ['graph-inferred', 'Workflow inferred'], ['runtime-parts-omitted', 'Workflow inferred'], + ['added-conditioning', 'Workflow inferred'], ['runtime-parts-omitted-and-added-conditioning', 'Workflow inferred'], ['recorded-run', 'Recorded run'], + ['embedded-parameters', 'Embedded parameters']]; function evidenceBadge(prompts) { const sources = new Set(Object.values(prompts).filter(role => role?.status === 'resolved').map(role => role.source)); return EVIDENCE_BADGES.find(([source]) => sources.has(source))?.[1] ?? null;