Feature/civitai metadata - #105
Closed
Mossbraker wants to merge 667 commits into
Closed
Mossbraker wants to merge 667 commits into
Mossbraker wants to merge 667 commits into
Conversation
…tract helpers Co-authored-by: Mossbraker <37157218+Mossbraker@users.noreply.github.com>
Co-authored-by: Mossbraker <37157218+Mossbraker@users.noreply.github.com>
Address review feedback: consolidate logic, clarify comments, extract helpers
…xx/ComfyUI_SaveImageWithMetaDataUniversal into non-descriptive-names-fix
…g, rename variables Co-authored-by: Mossbraker <37157218+Mossbraker@users.noreply.github.com>
Co-authored-by: Mossbraker <37157218+Mossbraker@users.noreply.github.com>
Updated with old too-long docstring to clarify the function's behavior with nested structures.
Remove unnecessary closing brace from the example.
Address code review feedback: clarify comments, fix duplicate handling, rename variables
…lpers Co-authored-by: Mossbraker <37157218+Mossbraker@users.noreply.github.com>
Refactor: pure functions, descriptive names, DRY sort helpers
Co-authored-by: Mossbraker <37157218+Mossbraker@users.noreply.github.com>
Fix truthiness checks and cycle detection in workflow metadata validation
Note: Not working with WebP images anymore. - validate_metadata.py now ignores helper nodes like `KSamplerSelect` when tracing sampler chains, so save nodes latch onto the real sampler (`SamplerCustomAdvanced` here). That restores model/LoRA/shift data in `expected_metadata`, which in turn re‑enables all the LoRA count/name/strength/hash checks for this workflow. - The “checks performed” total is recomputed after every validation pass, so the numeric summary now matches the verbose “Validation Details” list (no more “3 checks” when 20+ ran). - Added mandatory presence checks for the `Negative prompt` header (even when it’s blank in the workflow) and for the `Metadata generator version` field, giving the extra coverage you asked about. - Tightened the `Hashes` verification so every model/VAE referenced in the main parameter string must appear in the `Hashes` JSON and match; the existing LoRA/embedding logic already provides the per-item comparisons you enumerated. - These changes live entirely in validate_metadata.py. - validate_metadata.py now treats only real samplers as part of a save node’s lineage, so the large workflow picks up its LoRA stack/shift values and all associated checks. The same file gained richer metadata validation: the negative prompt header and metadata generator version are now required, and hashes/VAE/model fields must exist anywhere the workflow records those artifacts. - `_validate_hashes_summary` records separate checks for Hashes-entry presence and value agreement for every model/VAE/LoRA/embedding item, failures when summaries are missing, and still logs mismatch details. `_validate_required_field_pairs` enforces that each LoRA has name/hash/strength (plus model & VAE hashes, embedding hashes) and `_validate_hash_uniqueness` fails if any artifact hashes repeat. - Running `.venv\Scripts\python.exe -m compileall tests/tools/validate_metadata.py` completed without errors to sanity-check the edits. - WebP metadata decoding was the culprit: PIL sometimes exposes `img.info["parameters"]` as raw bytes for WebP outputs, and our reader used to ignore any non‑string values, leading to an empty metadata dict and the “No metadata found” error. - Added `MetadataReader._decode_text_value(...)` and now both PNG/WebP readers run everything through it, so byte chunks (UTF‑8/UTF‑16/latin‑1) are decoded before validation; other text keys inherit the same resilience. - Added a “Checks Per Image” section at the bottom of validate_metadata.py so every run now ends with lines like `✓ Large-Workflow-png_00001_.png (53 checks)`—handy for spotting outliers at a glance. - Fixed the metadata parser so it no longer drops custom `key: value` entries (extra metadata, user fields, etc.). The newline parser now accepts any colon-delimited key after the metadata block starts, and a new `_capture_additional_fields` pass scoops up remaining pairs in both newline and comma-separated formats. That removes the false negatives you saw for `custom_key`, `hello`, `custom_w`, and `custom_h`. - Updated the WebP reader to decode byte-valued text chunks, which restored metadata detection for WebP outputs created before the recent sampler changes. - Positive prompt validation now understands Flux dual-prompt captures: when a workflow emits `T5 Prompt` and `CLIP Prompt` instead of a classic `Positive prompt`, the validator treats that pair as satisfying the requirement (while still comparing each prompt against the workflow expectations). - LoRA/embedding field presence checks were tightened so model names, hashes, and strengths are all required, and a uniqueness check verifies that model/VAE/LoRA/embedding hashes don’t collide. - validate_metadata.py: filtered the “Checks Per Image” summary so entries whose filenames contain `without-meta` no longer appear; if every result is a control image, the section now shows a brief note explaining the skip. - validate_metadata.py: WebP metadata read now mirrors your manual approach. After the existing piexif path we call Pillow’s `getexif()` / `_getexif()` so UTF‑16 `UserComment` blocks—decoded with our updated helper—populate `metadata["parameters"]`. If that still fails, we scan the raw RIFF payload for a `parameters` substring so the validator can at least capture the text payload. PNG/WebP info iteration now treats the `Parameters` key case-insensitively, so mixed-case keys still hydrate the canonical entry. - `_decode_text_value` already prioritizes UTF‑16LE/BE when null bytes are present and strips embedded nulls; combined with the new WebP fallbacks, WebP EXIF user comments should decode the same way your snippet’s `decode_user_comment` does. - validate_metadata.py: WebP metadata read now mirrors your manual approach. After the existing piexif path we call Pillow’s `getexif()` / `_getexif()` so UTF‑16 `UserComment` blocks—decoded with our updated helper—populate `metadata["parameters"]`. If that still fails, we scan the raw RIFF payload for a `parameters` substring so the validator can at least capture the text payload. PNG/WebP info iteration now treats the `Parameters` key case-insensitively, so mixed-case keys still hydrate the canonical entry. - `_decode_text_value` already prioritizes UTF‑16LE/BE when null bytes are present and strips embedded nulls; combined with the new WebP fallbacks, WebP EXIF user comments should decode the same way your snippet’s `decode_user_comment` does.
- validate_metadata.py: added `_extract_parameters_from_binary` to recover metadata blocks directly from PNG/WebP bytes, including UTF‑16 sequences prefixed with `UNICODE`. Both PNG and WebP fallbacks now call this helper, so parameters embedded as UTF‑16 in WebP EXIF chunks (what you see when opening the file in Notepad) are decoded and fed back into the validator. Existing PNG logic benefits too when Pillow fails to surface a text chunk.
…g hashes now follow the same sidecar-aware code path as every other artifact and log compute failures consistently. - Added `tests/test_formatters_embeddings.py::test_extract_embedding_hashes_create_sidecar` to confirm formatter-level hashing writes `.sha256` files and returns truncated hashes. - Added `tests/test_embeddings.py::test_prompt_scan_writes_embedding_sidecar` to ensure the prompt-augmentation flow creates full-length sidecars for workflow-discovered embeddings.
Co-authored-by: Mossbraker <37157218+Mossbraker@users.noreply.github.com>
Fix nested test function and add embedding metadata assertions
- Added test_trace.py to exercise the real tracing logic: BFS distance mapping, warning path when the start node is absent, sampler selection for far/near strategies, heuristic fallbacks when `SAMPLERS` lacks entries, strict validation for the “By node ID” mode, and deterministic filtering/sorting of captured inputs. A `fresh_trace_module` fixture reloads `saveimage_unimeta.trace` before each case so earlier suites’ monkeypatches can’t leak into these assertions. **Coverage** - trace.py line coverage jumped from 15 % to 81 % (per the latest pytest-cov run), meaning its key decision paths and heuristics are now protected by unit tests. **Tests** - `METADATA_TEST_MODE=1 pytest --maxfail=1 --cov=saveimage_unimeta --cov-report=term-missing` **Coverage Boost** - Added test_utils_deserialize.py to round-trip JSON captures, hit warning/error branches, and exercise `format_config` plus integer enum restoration; this lifted deserialize.py coverage from 45 % to 91 %. - Added test_defs_validators.py to assert positive/negative prompt detection (including regex-based encoders) and to verify the connection cache’s positive/negative memoization paths, raising validators.py coverage from 10 % to 90 %. - Retained the earlier test_trace.py additions so sampler traversal, heuristics, and input filtering continue to enjoy 81 % coverage. **Tests** - `METADATA_TEST_MODE=1 C:/StableDiffusion/.../.venv/Scripts/python.exe -m pytest --maxfail=1 --cov=saveimage_unimeta --cov-report=term-missing` - embedding helpers: test_hook.py exercises `saveimage_unimeta.hook`’s global state capture (ensuring `pre_execute` snapshots prompt/executor data and `pre_get_input_data` only updates the active save node), while test_utils_embedding.py validates `saveimage_unimeta.utils.embedding.get_embedding_file_path` across multi-directory searches, recursive traversal rejects, invalid-directory failures, and empty-result handling. - Pytest (with `METADATA_TEST_MODE=1`) now passes: `python -m pytest --maxfail=1 --cov=saveimage_unimeta --cov-report=term-missing`, covering 441 tests in ~7 s and raising overall coverage to ~67%. Let me know which module you’d like to target next—`capture.py` or the remaining `defs/ext/*.py` helpers still show the largest gaps.
- Added test_capture_fields_and_hashes.py to exercise `Capture.get_inputs`’ advanced rule paths (multi-field enumerations, prefix scanning, and hash-formatting guards). - Verified multi-field rules apply formatter output lists and propagate custom `source_tag` values. - Confirmed name/hash formatters skip hashing in both protected (`MODEL_NAME`) and non-path inputs while still transforming path-like values. This hits the hash gating branches that were previously uncovered. **Testing** - `METADATA_TEST_MODE=1 python -m pytest --maxfail=1 --cov=saveimage_unimeta --cov-report=term-missing` (444 passed, repo coverage ↑ to 67.5%, capture.py up ~1 point). Coverage for capture.py now includes the “fields/prefix” logic and the hash formatter guards that were previously untested. Natural next targets would be the remaining large uncovered sections in capture.py (e.g., selector error paths, Flux fallbacks beyond T5/CLIP) or the low-covered helper modules listed in the coverage report.
| Area | Tests Added | |------|-------------| | Selector error handling | KeyError, TypeError graceful skipping; list-value expansion; validation skip | | Weight dtype sanitization | Known tokens accepted, path-like rejected, numeric rejected | | Prompt normalization | `none` literal blanked; guidance/CFG float casting | | Sampler fallback | `inputs_before_this_node` recovery path | | Steps validation | Negative rejected, zero accepted | | LoRA records | Aggregate text filtering; invalid name rejection | | Hex hash detection | Valid/invalid cases | | gen_parameters_str | guidance_as_cfg kwarg; dual prompt labels; LoRA summary override | **Results** - Full suite: **464 tests passed** in ~7.5 s - Overall coverage: **68.56%** (↑ from 67.49%) - capture.py: **67%** (↑ from 63%; 559 uncovered → 55 fewer uncovered lines) The remaining uncovered sections in capture.py are mostly deep runtime branches (graph introspection fallbacks, multi-sampler segment formatting, error-logging guards) that require ComfyUI runtime context to trigger. Coverage Summary: lora.py (62%) rgthree.py (22%) impact.py (22%) easyuse_nodes.py (31%) show_text.py (44%) rules_view.py (46%)
… check the specific modules that were targeted: - lora.py: Was 62%, now 82% ✓ - rgthree.py: Was 22%, now 76% ✓ - impact.py: Was 22%, now 72% ✓ - easyuse_nodes.py: Was 31% (not shown separately as it wasn't in original report, but coverage improved through `CAPTURE_FIELD_LIST` testing) - show_text.py: Was 44%, now 97% ✓ - rules_view.py: Was 46%, now not shown (implying higher coverage) The key improvements: - **lora.py**: 62% → 82% (+20%) - **rgthree.py**: 22% → 76% (+54%) - **impact.py**: 22% → 72% (+50%) - **show_text.py**: 44% → 97% (+53%) Let me create a summary of what was added: I've created 4 new test files with **81 new tests** that improve coverage for the previously low-covered modules: 1. **test_utils_lora.py** (16 tests) - Tests for `coerce_first`, `parse_lora_syntax` (strict/legacy formats, multiple LoRAs, invalid strengths) - Tests for `build_lora_index`, `find_lora_info`, `resolve_lora_display_names` - Coverage for utils/lora.py: **62% → 82%** 2. **test_ext_rgthree_impact.py** (32 tests) - Tests for rgthree `get_lora_data`, `_parse_syntax`, `_get_syntax` (with caching), syntax selectors - Tests for Impact `_coerce`, `_parse`, `_extract` (with caching), LoRA selectors - Coverage for defs/ext/rgthree.py: **22% → 76%** - Coverage for defs/ext/impact.py: **22% → 72%** 3. **test_ext_easyuse.py** (17 tests) - Tests for `get_lora_data_stack`, loraStack selectors (name, hash, strength) - Tests for simple vs advanced modes, toggle handling, num_loras limits - Tests for CAPTURE_FIELD_LIST and SAMPLERS dict structure 4. **test_nodes_show_and_rules.py** (16 tests) - Tests for ShowText `notify()` with workflow persistence, warning logging for malformed input - Tests for ShowGeneratedUserRules `show_rules()` with file reading, error handling - Coverage for nodes/show_text.py: **44% → 97%** - **Tests**: 464 → 545 (+81 tests) - **Total Coverage**: 68.56% → 71% (+2.44%) - All tests pass with lint clean
Address Copilot review 4162743880 on PR #135 noting that is_node_connected._cached_prompt holds a strong reference to the prompt dict. Both alternatives the bot suggested are rejected for concrete correctness reasons: - id(prompt): CPython freely reuses freed dict addresses, so a user rewiring a workflow without changing node count could collide with the cached id and silently return stale cache entries -- corrupting metadata for every saved image in that session. - weakref.ref(prompt): dict does not support weak references, so this is not viable without wrapping the prompt at every call site. The retained reference is bounded to one dict, overwritten on the next call with a different prompt. ComfyUI keeps the active prompt alive for the duration of execution, so this attribute does not meaningfully extend the prompt's lifetime. Comment-only change; no behavior delta.
…improvements Enraged antelopepr/bugfixes and improvements
…ort and clarify token usage Co-authored-by: Copilot <copilot@github.com>
fix: enhance tooltip for filename prefix to include subdirectory supp…
…ue pairs and improve input handling Co-authored-by: Copilot <copilot@github.com>
…traMetaDataUniversal Co-authored-by: Copilot <copilot@github.com>
…ll request head repository and head ref instead of assuming github.head_ref exists in the base repo. That fixes the fork-PR case that was producing the false negative. Co-authored-by: Copilot <copilot@github.com>
…a.py for issue 63
Fix issue 63
feat: add advanced toggle for suppressing missing class log and model…
chore: bump version to 1.4.3 and update changelog/release notes
- Guard Trace.trace against non-string node ids from V3 subgraph links - Guard validators._get_node_id_list with _is_link_input for subgraph links - Add regression tests for dict-link traversal
Refactor input field handling to use _is_link_input for validation.
Updated test to ensure BFS traversal skips list-of-dicts inputs instead of dict links.
…k-crash fix: Use the existing shared `_is_link_input` predicate instead of an ad-hoc `isinstance(value, list)` check, so non-link list values are skipped rather than hashed
chore: bump version to 1.4.4 and update changelog/release notes
Add utils/redaction.py: bounded, non-mutating workflow-metadata redaction (API keys, tokens, passwords, bearer credentials, absolute paths, control chars). Add utils/pathsafety.py: safe filename normalization for the expanded filename template (absolute paths, traversal, reserved Windows names, invalid chars). Wire both into SaveImageWithMetaDataUniversal: new 'sanitize_metadata' toggle (default on) redacts embedded workflow JSON; path safety is always-on after token expansion. Add unit tests for both modules.
Broaden sensitive-key coverage (client_secret, api_token, private_key, secret_key, auth_token, passwd) and harden filename sanitizer (reserved-name stem strip, truncation cleanup). Document the new toggle and filename behavior in README, CHANGELOG, and a new docs/SECURITY_REDACTION_AND_PATH_SAFETY.md; correct the Python floor to 3.10. Add save_images-level integration tests for the sanitize_metadata toggle (on/off/error-fallback) and combine the redaction import.
Update markdown.instructions.md to explicitly allow unbounded line lengths and stop hard-wrapping; reflow the affected docs (SECURITY_REDACTION_AND_PATH_SAFETY.md, README, CHANGELOG) onto single logical lines.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.