diff --git a/.github/branch-protection.json b/.github/branch-protection.json index 985d4116..4aca6925 100644 --- a/.github/branch-protection.json +++ b/.github/branch-protection.json @@ -3,17 +3,10 @@ "strict": false, "contexts": [ "py3.12 on ubuntu-latest", - "py3.13 on ubuntu-latest", - "py3.14 on ubuntu-latest", - "py3.12 on windows-latest", - "py3.13 on windows-latest", - "py3.14 on windows-latest", - "py3.12 on macos-latest", - "py3.13 on macos-latest", - "py3.14 on macos-latest", "coverage gate (85%)", "black and flake8", "frontend modules", + "all tests", "viewerjs", "real ComfyUI compatibility", "bandit (high severity)" diff --git a/.github/workflows/portability.yml b/.github/workflows/portability.yml new file mode 100644 index 00000000..75122ab0 --- /dev/null +++ b/.github/workflows/portability.yml @@ -0,0 +1,55 @@ +name: Windows and macOS + +permissions: + contents: read + +# Non-blocking portability runs (#420): never on pull requests or in the merge queue, and no context +# here is a required check. A red run is fixed at its cause before the workspace cutover (#334). +on: + push: + branches: [main] + schedule: + - cron: "17 3 * * *" + workflow_dispatch: + +# Every run gets its own group (run_id), so no main commit's run is ever cancelled or dropped as pending. +concurrency: + group: ${{ github.workflow }}-${{ github.run_id }} + cancel-in-progress: false + +env: + PYTHONFAULTHANDLER: "1" + +# Time limit rule as in test.yml: ceil(1.5 x the slowest of three recent green runs on main) + 3 +# minutes, never under 5. Measured 2026-10-26 in test.yml on runs 40257217681 (3be0ada), +# 40240706568 (fb9134f), 40228167328 (f9835aa), before these legs moved here in #420: +# slowest py3.10 on windows-latest: 12m43s / 15m54s / 14m53s -> 27 min +# (macos 6m14s-8m30s, windows 11m42s-15m54s) +jobs: + test: + name: py${{ matrix.python-version }} on ${{ matrix.os }} + runs-on: ${{ matrix.os }} + timeout-minutes: 27 + strategy: + fail-fast: false + matrix: + os: [windows-latest, macos-latest] + python-version: ["3.12", "3.13", "3.14"] + + steps: + - uses: actions/checkout@v7 + + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v7 + with: + python-version: ${{ matrix.python-version }} + cache: pip + cache-dependency-path: requirements-dev.txt + + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install -r requirements-dev.txt + + - name: Run tests + run: python -m tests.run_suite -v diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 43bf3d99..85194d1a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -28,10 +28,18 @@ env: # py matrix, slowest py3.10 on windows-latest: 12m43s / 15m54s / 14m53s -> 27 min # (ubuntu 5m27s-6m27s, macos 6m14s-8m30s, windows 11m42s-15m54s) # 3.10 and 3.11 left the matrix and 3.14 joined it in #356; re-measure after three green main runs. +# Windows and macOS moved to portability.yml (#420), which keeps this 27-minute limit; this +# Linux-only matrix keeps it too until it is re-measured. # real ComfyUI compatibility: 13m32s / 8m55s / 11m46s -> 24 min # coverage gate (85%): 7m26s / 7m22s / 7m54s -> 15 min # perf budget 30s / 29s / 27s, black and flake8 39s / 37s / 39s, # frontend modules 23s / 19s / 29s -> 5 min (the floor) +# all tests: a result check without setup, 5 min (the floor) +# +# Pull requests run a fast subset (#420): Python 3.12 on ubuntu-latest, coverage, lint and frontend +# modules. The merge queue (merge_group), pushes to main and manual runs add Python 3.13 and 3.14 on +# Linux and the real ComfyUI job. "all tests" is the required gate that holds the queue to that set. +# Windows and macOS run in portability.yml (push to main, nightly, on demand) and never gate a merge. jobs: test: name: py${{ matrix.python-version }} on ${{ matrix.os }} @@ -40,8 +48,8 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, windows-latest, macos-latest] - python-version: ["3.12", "3.13", "3.14"] + os: [ubuntu-latest] + python-version: ${{ github.event_name == 'pull_request' && fromJSON('["3.12"]') || fromJSON('["3.12", "3.13", "3.14"]') }} steps: - uses: actions/checkout@v7 @@ -63,6 +71,9 @@ jobs: comfyui: name: real ComfyUI compatibility + # Merge queue, main and manual runs only. A job skipped by its condition reports Success, so this + # required context never blocks a pull request; "all tests" fails if it is skipped anywhere else. + if: github.event_name != 'pull_request' runs-on: ubuntu-latest # Chromium and its system libraries come from the pinned Playwright image (tag = @playwright/test in # tools/vendor/package-lock.json; tests/vendor/browser-image.test.mjs checks it). The runner install @@ -253,3 +264,32 @@ jobs: - name: Run JS tests run: node --test tests/js/*.test.js + + # The required context "all tests". A dropped matrix leg never reports a check at all, so the eight + # legs that pull requests skip cannot be required by name; this gate requires them instead. Every + # job must succeed; real ComfyUI must be skipped on a pull request and succeed on every other event. + all-tests: + name: all tests + needs: [test, comfyui, coverage, lint, js-test] + if: always() + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Require every test job for this event + env: + EVENT_NAME: ${{ github.event_name }} + TEST_RESULT: ${{ needs.test.result }} + COMFYUI_RESULT: ${{ needs.comfyui.result }} + COVERAGE_RESULT: ${{ needs.coverage.result }} + LINT_RESULT: ${{ needs.lint.result }} + JS_RESULT: ${{ needs.js-test.result }} + run: | + queue_only=success + if [ "$EVENT_NAME" = pull_request ]; then queue_only=skipped; fi + echo "event: $EVENT_NAME (real ComfyUI must be $queue_only)" + echo "test matrix: $TEST_RESULT" + echo "comfyui: $COMFYUI_RESULT" + echo "coverage: $COVERAGE_RESULT" + echo "lint: $LINT_RESULT" + echo "js-test: $JS_RESULT" + test "$TEST_RESULT" = success && test "$COVERAGE_RESULT" = success && test "$LINT_RESULT" = success && test "$JS_RESULT" = success && test "$COMFYUI_RESULT" = "$queue_only" diff --git a/.github/workflows/viewerjs-check.yml b/.github/workflows/viewerjs-check.yml index 8582a67d..783e1b92 100644 --- a/.github/workflows/viewerjs-check.yml +++ b/.github/workflows/viewerjs-check.yml @@ -165,6 +165,8 @@ jobs: browser-workflows: name: page workflows (${{ matrix.browser }}, shard ${{ matrix.shard }}/2) + # Merge queue, main and manual runs only (#420); pull requests run the Chromium/touch coverage shards. + if: github.event_name != 'pull_request' runs-on: ubuntu-latest # Layout comparisons depend on the fonts each browser falls back to, so browser jobs run in the # same pinned Playwright image as local runs (tag = @playwright/test in tools/vendor/package-lock.json; @@ -219,8 +221,9 @@ jobs: retention-days: 14 # Branch protection requires the exact context "viewerjs". This job keeps that - # name and fails unless every frontend job above succeeded (every shard, the - # coverage merge and every browser shard), including when one was cancelled or skipped. + # name and fails unless every coverage shard and the coverage merge succeeded and + # the browser shards succeeded, or, on a pull request only, were skipped (#420). + # Any failure, cancellation or other skip fails it. viewerjs: needs: [frontend-coverage, frontend-coverage-merge, browser-workflows] if: always() @@ -229,11 +232,15 @@ jobs: steps: - name: Require every frontend job to succeed env: + EVENT_NAME: ${{ github.event_name }} COVERAGE_RESULT: ${{ needs.frontend-coverage.result }} MERGE_RESULT: ${{ needs.frontend-coverage-merge.result }} WORKFLOWS_RESULT: ${{ needs.browser-workflows.result }} run: | + queue_only=success + if [ "$EVENT_NAME" = pull_request ]; then queue_only=skipped; fi + echo "event: $EVENT_NAME (browser-workflows must be $queue_only)" echo "frontend-coverage: $COVERAGE_RESULT" echo "frontend-coverage-merge: $MERGE_RESULT" echo "browser-workflows: $WORKFLOWS_RESULT" - test "$COVERAGE_RESULT" = success && test "$MERGE_RESULT" = success && test "$WORKFLOWS_RESULT" = success + test "$COVERAGE_RESULT" = success && test "$MERGE_RESULT" = success && test "$WORKFLOWS_RESULT" = "$queue_only" diff --git a/README.md b/README.md index 7a918ef9..6142e6fc 100644 --- a/README.md +++ b/README.md @@ -56,7 +56,7 @@ Both nodes include the complete PromptManager feature set: - **๐Ÿ“Š Analytics**: Track prompt usage patterns and effectiveness over time - **๐Ÿ”ฌ Workflow Analysis**: Extract and analyze ComfyUI workflow data from PNG images - **๐Ÿ“‹ Metadata Viewer**: Standalone tool for analyzing ComfyUI-generated images -- **๐Ÿ› ๏ธ System Management**: Built-in diagnostics, backup/restore, and maintenance tools +- **๐Ÿ› ๏ธ System Management**: Built-in diagnostics, backup/restore, and maintenance tools in System - **๐Ÿท๏ธ AI AutoTag**: Automatically tag your image collection using WD14 or JoyCaption vision models - **๐Ÿ”— LoRA Manager Integration**: Import LoRA metadata and preview images from [ComfyUI-Lora-Manager](https://github.com/willmiao/ComfyUI-Lora-Manager) @@ -206,14 +206,11 @@ match Classic and Focused. - **Responsive Design**: Works on desktop, tablet, and mobile devices - **Real-time Search**: Instant search results as you type - **Bulk Operations**: Edit multiple prompts simultaneously with batch tools -- **Settings Panel**: Configure behavior and display options -- **System Management**: Complete toolset for database operations: +- **Settings and System tools live in System**: the dashboard's **Settings** opens System โ€บ Overview, **Maintenance** opens System โ€บ Maintenance, **Backup DB** and **Restore DB** open System โ€บ Backup & export, and **Diagnostics** and **View Logs** open System โ€บ Diagnostics & logs, each in a new tab. System reviews every destructive operation before it runs it as an Activity job. +- **Page options**: the list header's **Page options** keeps the prompt list's Infinite Scrolling default +- **Dashboard tools**: - **๐Ÿ” Metadata Viewer**: Launch standalone PNG analysis tool - - **๐Ÿ“ธ Scan Images**: Bulk scan output directories for ComfyUI images - - **๐Ÿ’พ Backup/Restore**: Database backup and recovery operations - - **๐Ÿ”ง Maintenance**: Database optimization and cleanup tools including orphaned prompt removal - - **๐Ÿ“‹ Logs**: Comprehensive logging system with download capability - - **๐Ÿ” Diagnostics**: System health checks and troubleshooting + - **๐Ÿ“ธ Scan Images**: Bulk scan output directories for ComfyUI images (with a quick backup first) - **Export Tools**: Download prompts and metadata in various formats - **Progress Tracking**: Real-time progress indicators for long operations - **Error Handling**: Graceful error display with recovery suggestions @@ -333,14 +330,11 @@ The comprehensive gallery system provides complete access to all your ComfyUI ou #### **โš™๏ธ Gallery Settings** -Configure your viewing experience in the settings panel: +The gallery's **Settings** opens System โ€บ Appearance in a new tab: image quality, details on cards, collapsed viewer details, video autoplay/mute/loop, metadata loading and file paths are set there and apply to this page too. -- **Display Settings**: Default view, items per page, grid columns, media info, infinite scrolling -- **Performance Settings**: Image quality, lazy loading -- **Video Settings**: Autoplay, mute by default, loop videos -- **Thumbnail Management**: Generate optimized thumbnails for faster loading (runs on the configured worker threads; a run that stops at the per-run cap tells you how many remain) -- **Find Duplicate Images**: Hashes every file on the worker threads with a progress bar; close the window and it keeps running, results wait in a pill -- **Rescan Output Folder**: Drops the server-side listing cache and rewalks the folders +The gallery's **Page options** keep the page's own settings: the thumbnail check at startup, lazy loading, items per page, grid columns, infinite scrolling, debug mode and API timeout. Reset puts back only these. + +Thumbnail generation, the server thumbnail and browser cache clears, refreshing the file listing and finding duplicate images are in System โ€บ Sources & scans and System โ€บ Maintenance (Page options links both); each runs as an Activity job you can follow after closing the page. #### **๐Ÿ–ผ๏ธ Image Viewing Experience** @@ -398,7 +392,7 @@ Import existing ComfyUI images into your database: - Any errors or issues encountered 5. **Access imported data** through the normal gallery interface -The scan reads PNG text chunks and A1111-style parameters, and, when `ffprobe` is installed, the prompt ComfyUI's video savers embed in MP4/MOV/WebM files. Files are read on the number of **worker threads** set in Settings โ†’ Performance (default: half your cores), which makes a large library several times faster. +The scan reads PNG text chunks and A1111-style parameters, and, when `ffprobe` is installed, the prompt ComfyUI's video savers embed in MP4/MOV/WebM files. Files are read on the number of **worker threads** set in System โ€บ Sources & scans (default: half your cores), which makes a large library several times faster. ### ๐Ÿท๏ธ AI AutoTag @@ -414,7 +408,7 @@ Automatically tag your entire image collection using AI vision models: 1. **AutoTag (Batch Mode)**: Tag your entire collection automatically - Click **"๐Ÿท๏ธ AutoTag"** in the admin dashboard - - Choose your model type (FP16 or GGUF) + - Choose your model type (FP16 or GGUF). A model that is not downloaded yet shows **Manage in System**: download (or unload) it in System โ€บ Auto-tagging, then reopen the dialog - Select how to handle already-tagged images: - **Skip images with existing tags**: Preserve your meticulous manual tagging work - **Re-tag all images**: Overwrite existing tags with fresh AI analysis @@ -445,7 +439,7 @@ Adjust the system prompt to match your tagging style: #### **Requirements** - Sufficient VRAM for the chosen model (FP16 requires more, GGUF is lighter) -- Models are downloaded automatically on first use +- Models are downloaded in System โ€บ Auto-tagging - Feature requests and improvements are welcome! ### ๐Ÿ“ Folder Filter @@ -470,18 +464,18 @@ If you use [ComfyUI-Lora-Manager](https://github.com/willmiao/ComfyUI-Lora-Manag #### **Enabling the Integration** -1. Open the admin dashboard and click **Settings** -2. Scroll to the **Integrations** section โ€” PromptManager will auto-detect if LoRA Manager is installed +1. Open System โ€บ Integrations (the admin dashboard's **Settings** opens System) +2. PromptManager auto-detects whether LoRA Manager is installed ![LoRA Enabled](images/pm-lora-enabled.png) 3. Toggle the **LoraManager** switch to enable 4. Optionally enable **Auto-inject trigger words** to automatically append LoRA trigger words when `` is detected in your prompts -5. Click **Save Settings** (requires a ComfyUI restart to take effect) +5. Click **Save preferences** (requires a ComfyUI restart to take effect) #### **Importing LoRA Data** -Click **Import LoRA Data** in the settings panel to start the import. A progress popup shows real-time status as each LoRA is processed: +Click **Review import** in System โ€บ Integrations; Start runs the import as an Activity job: - Reads LoRA Manager's own settings the way LoRA Manager does: from the folder in `LORA_MANAGER_SETTINGS_DIR`, from the plugin's `settings.json` in portable mode, or from your user config folder, and only the keys that say where its files are - Finds each LoRA's metadata in both storage modes: beside the model (the default) or in the central sidecar folder, and imports only LoRAs whose model file still exists in a LoRA folder (the review says how many metadata files it skipped) @@ -502,7 +496,7 @@ A CivitAI API key is **required if your library contains NSFW LoRAs** โ€” CivitA To add your key: 1. Get your API key from [civitai.com/user/account](https://civitai.com/user/account) -2. Paste it into the **CivitAI API Key** field in the Integrations settings +2. Paste it into the **CivitAI API key** field in System โ€บ Integrations 3. Save and re-import to download any previously skipped images The key is sent only over HTTPS to CivitAI's own hosts. CivitAI serves images through a redirect to its storage host; PromptManager follows such a redirect only over HTTPS to a host on CivitAI's own `civitai.com`, `civitai.red` or `civitai.green` domains, at most three hops, and never sends the key with it. When downloads fail, the import's status line says how many; when none could be downloaded, it starts with a warning. The server log has one warning per failed image with the reason (HTTP status, refused redirect, size cap, unreadable image, timeout or network error). @@ -517,23 +511,20 @@ The comprehensive web interface provides: - **Filter Options**: Filter by category, tags, rating, and date - **Bulk Operations**: Select multiple prompts for batch editing - **Export Tools**: Download your prompt collection -- **Settings Panel**: Configure auto-save and display options -- **Diagnostics**: System health checks and troubleshooting +- **Settings**: System (new tab) for settings, maintenance, backups and diagnostics; **Page options** for the page's own display settings ### ๐Ÿ”ง Database Maintenance -Keep your database optimized with built-in maintenance tools: +Keep your database optimized in System โ€บ Maintenance (the admin dashboard's **Maintenance** opens it in a new tab): -1. **Open the admin dashboard** and click **"๐Ÿ”ง Maintenance"** -2. **Select maintenance operations**: - - **Remove Duplicates**: Find and merge duplicate prompts while preserving images - - **Vacuum Database**: Optimize database file size and performance - - **Clean Orphaned Images**: Remove image records for missing files - - **๐Ÿšฎ Prune Orphaned Prompts**: Remove prompts that have no linked images - - **Check Consistency**: Verify database integrity and relationships - - **Database Statistics**: Generate detailed database reports -3. **Run selected operations** and monitor progress in real-time -4. **Review results** showing detailed information about each operation +- **Duplicate prompt text**: merge duplicate prompts while preserving images +- **Optimize database**: reclaim unused SQLite space +- **Missing-file links**: remove image records for missing files +- **Unused prompt components**: remove prompts nothing uses +- **Run health checks**: verify database integrity and relationships (the Health report) +- **Database size and prompt counts**: System โ€บ Diagnostics & logs (Database) and the Overview's Statistics + +Each operation opens a review that shows what it will change and the safety backup it takes, then runs as an Activity job. ### Database Location @@ -835,8 +826,8 @@ Before upgrading or changing this setting: 1. Identify the database your current installation actually uses and confirm its library contents. Check both the configured path and the prior fallback `/prompts.db`; keep track of which existing file you intend to use. -2. Preserve a consistent backup of the active database with the dashboard's - Backup/Restore backup action or SQLite's online backup API. Do not copy only +2. Preserve a consistent backup of the active database with + System โ€บ Backup & export's Back up or SQLite's online backup API. Do not copy only the main file of a live SQLite database: committed data may still be in its `-wal` file. For a filesystem copy, stop ComfyUI cleanly first and preserve any remaining `-wal` file together with the main database. diff --git a/TESTING.md b/TESTING.md index ea0280c7..ea6eefda 100644 --- a/TESTING.md +++ b/TESTING.md @@ -181,10 +181,12 @@ check, so run it before pushing a new or deleted frontend file instead of waiting for the `frontend coverage` jobs. The `viewerjs` check runs on every PR, including backend-only changes. It is an -aggregate job that passes only when every frontend job succeeds: the three -`frontend coverage (shard N/3)` jobs, `frontend coverage (merge)`, and the four -`page workflows (firefox|webkit, shard N/2)` jobs. A failed, cancelled or skipped -job fails it. Every browser job runs with `PM_TEST_WORKERS: 3` (the public +aggregate job. It requires the three `frontend coverage (shard N/3)` jobs and +`frontend coverage (merge)` to succeed on every event. The four +`page workflows (firefox|webkit, shard N/2)` jobs run only in the merge queue, +on `main` and on `workflow_dispatch` (see "Pull requests and the merge queue"). +`viewerjs` requires them to be skipped on a pull request and to succeed +everywhere else. Any other failed, cancelled or skipped job fails it. Every browser job runs with `PM_TEST_WORKERS: 3` (the public runners have 4 vCPUs). Each coverage shard runs `test:coverage:shard -- N/3` (shard 1 also runs the node units and the viewer suite, shard 3 the vendor pins and tooling tests) and uploads its raw V8 files as `coverage-raw-N` (kept 3 @@ -271,6 +273,13 @@ gh run view --json jobs # shows a job that alr gh run view --job --log-failed # that job's failed log, at once ``` +Python 3.13/3.14, real ComfyUI and Firefox/WebKit failures first show up in the +merge queue, on the merge-group commit rather than the +pushed one. Find that run with `gh run list --event merge_group --workflow test.yml` +(or `viewerjs-check.yml`) and match its head branch +(`gh-readonly-queue/main/pr--...`). Windows and macOS failures show up only +after the merge, in `gh run list --workflow portability.yml`. + Start fixing as soon as a failed log shows the cause, but push the fix only after the run for that commit has finished: pull-request runs use `cancel-in-progress`, so a push cancels the running jobs and can hide a second failure. @@ -313,6 +322,26 @@ from the tests' package unload (`cleanup_package`), and it removes its own ComfyUI-Manager restart through `os.execv`, a closed Windows console, or ComfyUI Desktop stopping its server. SQLite's WAL journal covers those. +Jobs running at exit (#376): before any `atexit` callback, Python joins every +executor thread (`concurrent.futures` registers that join with +`threading._register_atexit`). The package registers its own callback there +(`utils/exit_signal.install`, after `concurrent.futures`, so it runs first): +it sets the exit signal and kills the running encoder processes +(`shutdown_encoders`, the ffmpeg that never sees Ctrl+C on Windows). The call in +flight then stops at its next safe point: `Job.cancelled` is True, so every +check a job already makes between files stops it, `map_parallel` starts no +further item (`ExitRequested`), the scan walk stops between folders, LoRA +import staging stops before its next sidecar (also for the legacy scan) and a +tagging model download stops before its next chunk where huggingface_hub takes +`tqdm_class` (1.x removes its temp file; 0.x `hf_hub_download` does not take it +and finishes the file). Xet transfers ignore that checkpoint, so the hook also +calls hub's own `abort_xet_session` when huggingface_hub is loaded. The +trigger-word cache load (small JSON sidecars) has no checkpoint. No check sits +inside a library write. That hook is +CPython-internal; without it (or when it is refused) `install` returns False and +exit waits for the call in flight, as before. `tests/test_exit_cancel.py` runs a +child process with three such calls running and requires its exit within 15 s. + Stop contract: `PromptTracker.shutdown(timeout)`, `ImageGenerationHandler.shutdown(timeout)` (restartable), `ImageGenerationHandler.close(timeout)` (final: `schedule()` refuses and a @@ -519,29 +548,95 @@ Before changing paths, SQLite, streams, restore or cleanup: open with `FirstWriterHold`) and give peers that must all succeed the ordinary busy timeout; cross-thread handoffs wait on events with generous ceilings, never a short wall clock (#242). -- Run native Windows CI early for these changes and include a Windows impact and - evidence note in the PR. All twelve OS/Python combinations and every applicable +- Run native Windows CI early for these changes. Pull requests and the merge + queue run only Linux, so start Windows and macOS on the branch with + `gh workflow run portability.yml --ref `. + Include a Windows impact and evidence note in the PR. All six Windows/macOS + combinations in that run, every Linux leg and every applicable exact-head check must pass before merge. Portable `ntpath`/`PureWindowsPath` controls do not establish native filesystem, locking or cleanup success. +## Pull requests and the merge queue + +Each PR runs the full matrix once, in the merge queue (#420). The event decides +what runs: + +| Job | `pull_request` | `merge_group`, push to `main`, `workflow_dispatch` | +| --- | --- | --- | +| `py3.12 on ubuntu-latest` | runs | runs | +| `py3.13 on ubuntu-latest`, `py3.14 on ubuntu-latest` | not created | run | +| `real ComfyUI compatibility` | skipped | runs | +| `coverage gate (85%)`, `black and flake8`, `frontend modules`, `bandit (high severity)` | run | run | +| `frontend coverage (shard N/3)`, `frontend coverage (merge)` (vendor pins and tooling tests on shard 3) | run | run | +| `page workflows (firefox\|webkit, shard N/2)` | skipped | run | +| `all tests`, `viewerjs` (gates) | run | run | +| `perf budget` | skipped | push and dispatch only (not in the queue) | +| `py3.12-3.14 on windows-latest/macos-latest` (`portability.yml`) | not run | push to `main`, nightly, `workflow_dispatch` only; never required | + +GitHub has one required set for pull requests and the queue. A job skipped by +its `if:` reports Success. A dropped matrix leg never reports, so it would stay +pending. That is why `all tests` (in `test.yml`) and `viewerjs` carry the +guarantee: +- On a pull request they require the subset to succeed and the queue-only jobs + to be skipped. +- On every other event they require every job to succeed. + +So a merge group cannot turn green unless all three Linux Python legs, real +ComfyUI and the four browser shards passed. + +Windows and macOS (Python 3.12-3.14) are out of the blocking path for now +(maintainer, 2026-10-28). They run in `portability.yml` on every push to `main`, +nightly and on demand, and no context there is required. A red portability run +is still a failure: fix it at its cause, not by rerunning, and have every one +green before the workspace cutover (#334). `tests/vendor/ci-workflow.test.mjs` pins the +event split and runs both gate scripts for each event. + ## Required merge checks -`.github/branch-protection.json` records the required checks: all twelve Python/OS -combinations, Python coverage, formatting, frontend modules, `viewerjs`, `real ComfyUI compatibility`, -and `bandit (high severity)`. -The branch must be current with main, and administrators follow the same checks. +`.github/branch-protection.json` records the required checks: +`py3.12 on ubuntu-latest`, `coverage gate (85%)`, `black and flake8`, +`frontend modules`, `all tests`, `viewerjs`, `real ComfyUI compatibility` and +`bandit (high severity)`. Each one reports on a pull request. +The same set lives in two places, and both must change together: the `main` +ruleset (25960526, which also holds the merge queue) and the classic branch +protection on `main`, which the manifest mirrors. GitHub requires the union of +the two, so a context left in either one blocks every PR that does not report it. It does not require an additional human approval. A repository administrator applies the configuration after the new workflow is passing, then verifies it: +```sh +gh api repos/ComfyAssets/ComfyUI_PromptManager/rulesets/25960526 \ + | jq '.rules[] | select(.type == "required_status_checks") | [.parameters.required_status_checks[].context]' +``` + +A PUT replaces the whole `rules` array, so edit the current ruleset in place. +Keep the `deletion`, `non_fast_forward` and `merge_queue` rules, and give every +context `integration_id` 15368 (GitHub Actions), so no other integration can +post a required check. Save the current ruleset first; it is the rollback: + +```sh +gh api repos/ComfyAssets/ComfyUI_PromptManager/rulesets/25960526 > ruleset-before.json +jq '{name, target, enforcement, conditions, bypass_actors, rules: [.rules[] + | if .type == "required_status_checks" + then .parameters.required_status_checks = [$ARGS.positional[] | {context: ., integration_id: 15368}] + else . end]}' ruleset-before.json --args "py3.12 on ubuntu-latest" "..." > ruleset-new.json +gh api --method PUT repos/ComfyAssets/ComfyUI_PromptManager/rulesets/25960526 --input ruleset-new.json +``` + +Afterwards, read it back. Check that the `merge_queue` rule is still there and +that every context keeps its `integration_id`. Then apply the manifest to the +classic protection and read both back: + ```sh gh api --method PUT repos/ComfyAssets/ComfyUI_PromptManager/branches/main/protection \ --input .github/branch-protection.json gh api repos/ComfyAssets/ComfyUI_PromptManager/branches/main/protection/required_status_checks +gh api repos/ComfyAssets/ComfyUI_PromptManager/rules/branches/main ``` - -Update the manifest whenever required job names change. Keep the frontend check + Update the manifest whenever +required job names change. Keep the frontend check free of path filters so a required check is never silently skipped. ## Unified workspace @@ -794,6 +889,25 @@ unattributed stage, when it is a plain first stage: not a detailer, no prompt ov set, and no listed sampler in its own latent ancestry (else it stays unattributed) (`tests/test_resolver_saver_anchor.py`, shapes in `tests/saver_anchor_graphs.py` from the maintainer's `gpu_unlock_bench`, dataset and ANIMA runs; browser parity through the `anchor-*` cases, which carry `__file_name`). +Recorded run history (#386): both inspect routes (Images and a history occurrence) +ask `database.run_attribution.recorded_output` inside the file's stat window. It +answers only for a file that exactly one recorded run reported and that was written +during that run (the #342 rules): the one output node that run reported it from (none +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; 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`). `tools/inspection_corpus.py --root DIR --out JSONL [--workers N] [--baseline JSONL] [--summary TXT] [--code TREE]` measures a media folder read-only against the #240 @@ -2524,6 +2638,14 @@ production hub `#/overview` is unchanged (D28); its System card opens the pressed section button and disposes the previous section's scope, so its reads, listeners and dialogs never outlive it. +System is the home for settings and maintenance (#330 System S13). The legacy pages keep +their prompt list, tags, Scan Images, auto-tag batch and review and gallery browsing; their +Settings, Diagnostics, View Logs, Maintenance, Backup DB, Restore DB, the gallery's +thumbnail, cache, rescan and duplicate tools and the auto-tag model downloads open the +System section that owns them in a new tab (`legacy-system-links.spec.mjs`). Each page +keeps only its own settings in Page options. What left the legacy pages is listed in +"Legacy System UI removed by S13". + Drafts (`system-state.mjs`) live for the page session (Q10). Save writes the dirty browser fields, then the host fields (`POST /prompt_manager/settings` with `expected_revision`, #220), then the runtime fields. A failed domain keeps its drafts and @@ -4138,6 +4260,9 @@ auto-tag / review", which the Images selection bar owns, not a System section. A is not `covered` names none (`โ€”`). Test is the exact title of a test that runs, then its file. Anchors are line numbers on `main` at this PR. +S13 (#330) removed the legacy copy of the `covered` rows the legacy pages linked to System; +"Legacy System UI removed by S13" below lists every removed element against its row. + `tests/vendor/legacy-coverage.test.mjs` fails on a row with any of these problems: - no owner, or a status outside the three; - a malformed Native control cell; @@ -4218,7 +4343,11 @@ opens `workspace#/system/` in a new tab) and removes the legacy copy of a ca only when its row above is `covered`. One row per removed capability: Capability is the exact Capability cell of its coverage row, Page is `admin` or `gallery`, Removed ids and Removed methods list what went (`โ€”` for none), and Native test copies that row's Test -cell. An id that stays on a System link is not listed, only its handler. +cell. An id that stays on a System link is not listed, only its handler. The gallery's +Default View Mode left Page options too: System โ€บ Appearance's Default layout is the one +default-layout control. Nothing writes the legacy `defaultViewMode` any more; a stored +value still picks the view the legacy gallery opens in (grid otherwise), and Page options +Save and Reset keep it. `tests/vendor/legacy-coverage.test.mjs` fails when a row cites a capability that is not `covered` or copies another test, when an id or method is listed twice for one page, or @@ -4237,6 +4366,31 @@ legacy page workflows), not by this check. | Log files list and download | admin | `#downloadLogsBtn`, `#logFilesList` | `loadLogFiles()`, `downloadLogs()`, `downloadLogFile()` | each listed file downloads by its returned name and a link has no Download (workspace-system-diagnostics.spec.mjs) | | Clear/truncate logs | admin | `#clearLogsBtn` | `clearLogs()` | Clear all logs states both counts, confirms first and reads everything again (workspace-system-diagnostics.spec.mjs) | | Set log level (incl. CRITICAL), console logging | admin | `#updateLogConfigBtn`, `#setLogLevel`, `#consoleLogging` | `loadLogConfig()`, `updateLogConfig()` | runtime level (including CRITICAL) and console logging save and say they apply until the server restarts (workspace-system-diagnostics.spec.mjs) | +| Image scan directories add/remove | admin | `#scanPathsList`, `#addScanPathBtn` | `renderScanPaths()`, `addScanPath()`, `removeScanPath()`, `_collectScanPaths()` | Add source validates on the server, then marks the host draft dirty; Save persists (workspace-system-sources.spec.mjs) | +| Currently monitoring list + Refresh | admin | `#refreshMonitoringStatus`, `#monitoringStatus` | `updateMonitoringStatus()` | watching only with a live observer; restart pending otherwise (workspace-system-sources.spec.mjs) | +| Search results auto-hide `result_timeout` | admin | `#resultTimeout` | โ€” | host fields round-trip through /settings with revision (workspace-system.spec.mjs) | +| Web UI display mode | admin | `#webuiDisplayMode` | โ€” | host fields round-trip through /settings with revision (workspace-system.spec.mjs) | +| Worker threads | admin | `#workerThreads`, `#workerThreadsValue` | `renderWorkerThreads()` | worker range max is cpu_count (workspace-system-sources.spec.mjs) | +| LoraManager badge, enable, path, trigger words, CivitAI key, clear key | admin | `#loraDetectionBadge`, `#loraEnabled`, `#loraSettings`, `#loraManagerPath`, `#loraTriggerWords`, `#civitaiApiKey`, `#clearCivitaiKey` | `detectLoraManager()`, `_bindLoraEvents()`, `saveLoraSettings()` | LoRA rows show detected / enabled / cache-loaded separately (workspace-system-integrations.spec.mjs) | +| LoRA data import with progress | admin | `#loraImportModal`, `#loraImportBtn`, `#loraImportStatus`, `#loraImportPercent`, `#loraImportBar`, `#loraImportProcessed`, `#loraImportImported` | `runLoraImport()` | Review import shows replacement, blocked items and network effects with the backup box; Start opens the job (D107) (workspace-system-integrations.spec.mjs) | +| Maintenance cleanup_duplicates, vacuum, cleanup_orphaned_images, prune_orphaned_prompts | admin | `#maintenanceModal`, `#cleanup_duplicates`, `#vacuum`, `#cleanup_orphaned_images`, `#prune_orphaned_prompts`, `#runMaintenanceBtn`, `#maintenanceResults` | `showMaintenanceModal()`, `closeMaintenance()`, `runMaintenance()`, `renderMaintenanceResults()` | Apply reviewed operation closes, toasts, opens the job in Activity (workspace-system-maintenance.spec.mjs) | +| Maintenance check_hash_duplicates, check_consistency | admin | `#check_hash_duplicates`, `#check_consistency` | โ€” | Run health checks opens Health report and updates metric 4 (workspace-system.spec.mjs) | +| Maintenance statistics (database info) | admin | `#statistics` | โ€” | meta-grid shows real database and source status, the pinned Viewer.js version and the host renderers (workspace-system-diagnostics.spec.mjs) | +| Maintenance select all / clear all / run | admin | `#selectAllMaintenanceBtn`, `#clearAllMaintenanceBtn` | `selectAllMaintenance()`, `clearAllMaintenance()` | plan dialog shows the server's scope, protected, backup policy and media lines with the backup box (D100); Cancel writes nothing (workspace-system-maintenance.spec.mjs) | +| Database backup | admin | โ€” | `backupDatabase()` | Back up downloads through the api client and revokes the object URL (D103) (workspace-system-backups.spec.mjs) | +| Restore from file | admin | `#restoreModal`, `#restoreFileInput`, `#confirmRestore`, `#cancelRestore` | `showRestoreModal()`, `confirmRestore()` | Review restore stages the chosen file and shows the real summary with the backup box (D105) (workspace-system-backups.spec.mjs) | +| Gallery thumbnail generation | gallery | `#generateThumbnailsBtn`, `#thumbnailStatus`, `#thumbnailProgress`, `#progressText`, `#progressPercent`, `#cancelThumbnailsBtn`, `#progressBar`, `#progressDetails`, `#progressETA` | `generateThumbnails()`, `generateThumbnailsWithProgress()` | Start closes the dialog, toasts, and opens the job in Activity (workspace-system-sources.spec.mjs) | +| Default view mode, image quality | gallery | `#defaultViewModeSelect`, `#imageQualitySelect` | โ€” | defaults apply to a new visit but not to the current view or a saved view (workspace-system.spec.mjs) | +| Image info on cards; sidebar collapsed | gallery | `#showImageInfoToggle`, `#sidebarCollapsedByDefaultToggle` | โ€” | added rows reach their consumers (workspace-system.spec.mjs) | +| Video autoplay, mute, loop | gallery | `#videoAutoplayToggle`, `#videoMuteToggle`, `#videoLoopToggle` | โ€” | video defaults reach the viewer (workspace-system.spec.mjs) | +| Auto-load metadata, cache metadata, show file paths | gallery | `#autoLoadMetadataToggle`, `#cacheMetadataToggle`, `#showFilePathsToggle` | โ€” | metadata off shows an explicit load and loads once on request (workspace-system.spec.mjs) | +| Browser cache clear | gallery | `#clearCacheBtn` | `clearCache()` | Metadata cache reviews and clears only owned browser entries when there are some (D102) (workspace-system-maintenance.spec.mjs) | +| Clear server thumbnails | gallery | โ€” | โ€” | Thumbnail cache opens the S5 dialog (workspace-system-maintenance.spec.mjs) | +| Refresh file listing | gallery | `#rescanFolderBtn` | `rescanFolder()` | Refresh files calls /gallery/rescan and toasts its counts (workspace-system-sources.spec.mjs) | +| Duplicate image scan, review, delete | gallery | `#scanDuplicatesBtn`, `#dupScanPill`, `#dupScanPillText`, `#duplicatesModal`, `#closeDuplicatesBtn`, `#duplicatesScanStatus`, `#dupProgressText`, `#dupProgressBar`, `#duplicatesContent`, `#duplicatesList`, `#duplicatesFooter`, `#duplicatesCount`, `#selectAllBtn`, `#removeDuplicatesBtn` | `scanDuplicates()`, `attachDuplicateStream()`, `resumeDuplicateScan()`, `applyDuplicateEvent()`, `renderDuplicateProgress()`, `setDuplicateButton()`, `showDuplicatePill()`, `hideDuplicatePill()`, `openDuplicatesModal()`, `showDuplicatesModal()`, `hideDuplicatesModal()`, `displayDuplicates()`, `selectAllDuplicates()`, `removeDuplicates()` | Find duplicates starts a job; its view links Review groups; Keep radios build a duplicate_files plan (D101) (workspace-system-maintenance.spec.mjs) | +| Auto-tag model download | admin | `#autoTagDownloadModal`, `#downloadModelName`, `#downloadStatus`, `#downloadProgressPercent`, `#downloadProgressBar`, `#cancelDownloadBtn` | `downloadModel()`, `cancelDownload()` | Download starts the job and Activity shows it without Cancel (workspace-system-tagging.spec.mjs) | +| Auto-tag model download | gallery | `#autoTagDownloadModal`, `#downloadModelName`, `#downloadStatus`, `#downloadProgressPercent`, `#downloadProgressBar`, `#cancelDownloadBtn` | `downloadModel()`, `cancelDownload()` | Download starts the job and Activity shows it without Cancel (workspace-system-tagging.spec.mjs) | +| Unload model | gallery | โ€” | `unloadModel()` | Unload is enabled only while a model is loaded, and a held gate's refusal names the operation (workspace-system-tagging.spec.mjs) | ### Browser cache ownership (#222) @@ -4255,17 +4409,15 @@ page writes a key that is neither a preference nor inventoried, writes one it ca CacheStorage or IndexedDB; `the storage scan reports every way of writing browser storage` pins those patterns. It skips `web/lib` (third-party builds) and the pinned Toolkit build (`web/js/shared/toolkit`, never hand-edited, verified by `npm run check`; -its storage belongs to the Toolkit repository and a new pin is reviewed on its own). The -gallery's server thumbnail clear stays its own request and runs first: refused or -unreachable, it changes nothing and says so (the old code toasted success when the -request failed); refused browser storage is reported, never hidden -(`tests/js/legacy-refusals.test.js`; the browser case `the gallery cache clear keeps -preferences and other tools caches` in `workflows.spec.mjs`). +its storage belongs to the Toolkit repository and a new pin is reviewed on its own). Since +System S13 (#330) the legacy gallery no longer offers Clear cache: System โ€บ Maintenance's +Metadata cache and Thumbnail cache reviews own both parts (removal table above), and +`cache-ownership.js` serves the native review. -The legacy duplicates dialog's View Full now names its source (`?root=`, with the -path URL-encoded) when the gallery's output folder is a listed source; before, the serve -route opened the first source holding a file at that path -(`test_view_full_names_the_gallery_folder_source`). +The duplicates dialog's View Full named its source (`?root=`, with the path +URL-encoded) when the gallery's output folder is a listed source +(`test_view_full_names_the_gallery_folder_source`); S13 removed the legacy dialog, and the +route keeps that behaviour for System โ€บ Maintenance's duplicate review. ### Library location contract (#249 System S6b) diff --git a/__init__.py b/__init__.py index a398acfd..1365a2d9 100644 --- a/__init__.py +++ b/__init__.py @@ -355,6 +355,44 @@ async def _stop_background_work_on_cleanup(app): _stop_background_work() +def _kill_encoders_at_exit(): + """Run by the exit hook before Python joins the executor threads (#376): + a JPEG XL or ffmpeg encoder (no console on Windows, so Ctrl+C never + reaches it) is killed instead of being waited for.""" + from .py.api.thumbnail_encode import shutdown_encoders + + shutdown_encoders() + + +def _abort_downloads_at_exit(): + """Run by the exit hook (#376): huggingface_hub's Xet transfers ignore + the chunk checkpoint, so they are aborted the way hub aborts on Ctrl+C.""" + import sys + + if "huggingface_hub" not in sys.modules: + return # no download ever ran: import nothing at exit + from .py.autotag import abort_hub_downloads + + abort_hub_downloads() + + +try: + # Before the executor join: running jobs stop at their next file, item or + # chunk (#376). The teardown below still runs from atexit afterwards. + from .utils import exit_signal as _exit_signal + + _exit_signal.install(_kill_encoders_at_exit) + _exit_signal.install(_abort_downloads_at_exit) +except Exception as e: + try: + from .utils.logging_config import get_logger + + get_logger("prompt_manager.init").error( + f"Failed to register the exit signal: {e}" + ) + except Exception: + pass + try: atexit.register(_stop_background_work) from .py.config import server_instance as _teardown_server diff --git a/database/run_attribution.py b/database/run_attribution.py index b7d4ec58..50a671db 100644 --- a/database/run_attribution.py +++ b/database/run_attribution.py @@ -15,6 +15,10 @@ only when all of this holds: Anything else returns None: the caller leaves the file unlinked. Read-only. +``recorded_output`` applies the first two rules and returns, for the +inspector (#386), the one output node (save node) that run reported the file +from and the texts PromptManager recorded at execution per sampler. + Lookups are batched (``stored_run_prompts``): one parameterized query per chunk of at most ``LOOKUP_CHUNK`` distinct file names, matched in memory. The history schema is left exactly as it is (no extra index), so older builds can @@ -26,6 +30,7 @@ import os import sqlite3 import threading import weakref +from dataclasses import dataclass from typing import Any, Callable, Dict, Iterable, List, Optional try: @@ -45,7 +50,8 @@ LOOKUP_CHUNK = 500 _RUNS_FOR_NAMES = ( "SELECT DISTINCT o.filename, o.subfolder, o.root_key, o.path_style, o.run_id, " - "r.observed_dequeue_at, COALESCE(r.terminal_event_at, r.observed_callback_at) " + "r.observed_dequeue_at, COALESCE(r.terminal_event_at, r.observed_callback_at), " + "o.output_node_id " "FROM execution_run_outputs o JOIN execution_runs r ON r.run_id = o.run_id " "WHERE o.folder_type = 'output' AND o.filename IN ({marks})" ) @@ -138,9 +144,10 @@ def stored_run_prompt( def _runs_by_location(db: Any, names: List[str]) -> Optional[Dict[tuple, dict]]: - """``{(filename, subfolder, root_key, style): {run_id: (start, end)}}``. + """``{(filename, subfolder, root_key, style): {run_id: (start, end, nodes)}}``. - One query per ``LOOKUP_CHUNK`` names; None without history tables. + ``nodes`` is the set of output node ids that reported the location in that + run. One query per ``LOOKUP_CHUNK`` names; None without history tables. """ found: Dict[tuple, dict] = {} conn = db.model.get_connection() @@ -151,12 +158,49 @@ def _runs_by_location(db: Any, names: List[str]) -> Optional[Dict[tuple, dict]]: rows = conn.execute(query, chunk).fetchall() # nosec B608 except sqlite3.Error: return None # no history tables (a library without history) - for name, subfolder, root_key, style, run_id, began, ended in rows: + for name, subfolder, root_key, style, run_id, began, ended, node in rows: runs = found.setdefault((name, subfolder, root_key, style), {}) - runs[run_id] = (began, ended) + runs.setdefault(run_id, (began, ended, set()))[2].add(node) return found +def _recorded_runs( + db: Any, + image_paths: Iterable[str], + output_root: Optional[str], + path_style: Optional[str], +) -> Dict[str, tuple]: + """``{path: (run_id, output node ids)}`` for each file history attributes. + + A file is attributed when exactly one recorded run reported its location + and it was last written while that run executed. + """ + paths = list(dict.fromkeys(path for path in image_paths if path)) + if not paths or not output_root: + return {} + style = path_style or ("windows" if os.name == "nt" else "posix") + located = {} + for path in paths: + location = _location(path, output_root, style) + if location is not None: + located[path] = location + if not located: + return {} + names = sorted({filename for _key, _sub, filename in located.values()}) + runs = _runs_by_location(db, names) + if not runs: + return {} + attributed = {} + for path, (root_key, subfolder, filename) in located.items(): + reported = runs.get((filename, subfolder, root_key, style), {}) + if len(reported) != 1: + continue + ((run_id, (began, ended, nodes)),) = reported.items() + if _written_during(path, began, ended): + attributed[path] = (run_id, nodes) + return attributed + + def _named_prompt(db: Any, run_id: str) -> Optional[int]: contexts = db.get_execution_contexts(run_id).get("contexts") or [] named = { @@ -183,35 +227,115 @@ def stored_run_prompts( """ paths = list(dict.fromkeys(path for path in image_paths if path)) result: Dict[str, Optional[int]] = dict.fromkeys(paths) - if not paths or not output_root: - return result - style = path_style or ("windows" if os.name == "nt" else "posix") - located = {} - for path in paths: - location = _location(path, output_root, style) - if location is not None: - located[path] = location - if not located: - return result - names = sorted({filename for _key, _sub, filename in located.values()}) - runs = _runs_by_location(db, names) - if not runs: - return result prompts: Dict[str, Optional[int]] = {} - for path, (root_key, subfolder, filename) in located.items(): - reported = runs.get((filename, subfolder, root_key, style), {}) - if len(reported) != 1: - continue - ((run_id, (began, ended)),) = reported.items() - if not _written_during(path, began, ended): - continue + for path, (run_id, _nodes) in _recorded_runs( + db, paths, output_root, path_style + ).items(): if run_id not in prompts: prompts[run_id] = _named_prompt(db, run_id) result[path] = prompts[run_id] return result +@dataclass(frozen=True) +class RecordedOutput: + """What recorded history knows about the run that wrote one file (#386). + + ``saver_id`` is the one output node the run reported the file from (None + when several did). ``texts`` maps each sampler anchor of the run to the + effective texts PromptManager recorded at execution for its roles + (``{"positive": text or None, "negative": text or None}``); a role whose + text was only inferred from the graph is None. + """ + + saver_id: Optional[str] + texts: Dict[str, Dict[str, Optional[str]]] + + +def _recorded_texts(db: Any, run_id: str) -> Dict[str, Dict[str, Optional[str]]]: + texts = {} + for context in db.get_execution_contexts(run_id).get("contexts") or []: + evidence = context.get("evidence") or {} + roles = {} + for role in ("positive", "negative"): + values = context.get(role) or {} + proof = (evidence.get(role) or {}).get("effective") or {} + value = values.get("effective") + recorded = proof.get("provenance") == "recorded" + roles[role] = value if recorded and isinstance(value, str) else None + texts[context["anchor_id"]] = roles + return texts + + +def recorded_output( + db: Any, + image_path: str, + output_root: Optional[str], + path_style: Optional[str] = None, +) -> Optional[RecordedOutput]: + """The recorded run facts for ``image_path``, else None (#386). + + The file must be attributed to one run (``_recorded_runs``: one run + reported its location and it was written during that run). Read-only; it + never links anything. + """ + if not image_path: + return 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") + + +def recorded_output_lookup( + db: Any, *, close: bool = False, reference: Optional[dict] = None +) -> Callable[[Any], Any]: + """``lookup(path) -> RecordedOutput or None`` for the inspect routes (#386). + + Uses ComfyUI's output folder. Any failure (no history, a locked library) + is logged and names no node, so the file is judged from its own metadata. + ``close`` closes the calling thread's connection afterwards, for routes + that close theirs after reading. ``reference`` (a history occurrence's + output reference) spells the file as history recorded it: the route's + target is case-normalized on Windows, and history matches names exactly. + """ + + def lookup(path: Any) -> Optional[RecordedOutput]: + root = host_output_directory() + if reference is not None: + if reference.get("folder_type") != "output" or not root: + return None + path = os.path.join( + root, reference.get("subfolder") or "", reference.get("filename") or "" + ) + try: + return recorded_output(db, str(path), root) + 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() + + return lookup + + _SEALED_LISTENERS: List[weakref.WeakMethod] = [] _SEALED_LOCK = threading.Lock() diff --git a/py/api/admin.py b/py/api/admin.py index 047c6b8a..a96d07d2 100644 --- a/py/api/admin.py +++ b/py/api/admin.py @@ -34,6 +34,11 @@ try: except ImportError: from utils.parallel import map_parallel +try: + from ...utils import exit_signal +except ImportError: + from utils import exit_signal + try: from ...database.pair_ingestion import blank_positive, ingest_resolved_pair from ...database.run_attribution import ( @@ -118,6 +123,7 @@ def _collect_media_files(output_dirs, extensions): if not os.path.isdir(root): continue for current, dirnames, filenames in os.walk(root, followlinks=False): + exit_signal.checkpoint() # #376: stop between directories at exit depth = 0 if current == root else len(Path(current).relative_to(root).parts) dirnames[:] = sorted( d diff --git a/py/api/history.py b/py/api/history.py index 93d08a1d..9426ee0c 100644 --- a/py/api/history.py +++ b/py/api/history.py @@ -33,6 +33,7 @@ from .images import image_category_key try: from ...database.execution_history import HistoryEpochChanged from ...database.history_gallery import read_history_gallery + from ...database.run_attribution import recorded_output_lookup from ...database.history_reads import ( HistoryCaptureChanged, OutputPosition, @@ -48,6 +49,7 @@ try: except ImportError: from database.execution_history import HistoryEpochChanged from database.history_gallery import read_history_gallery + from database.run_attribution import recorded_output_lookup from database.history_reads import ( HistoryCaptureChanged, OutputPosition, @@ -776,6 +778,9 @@ class HistoryRoutesMixin: ), own, ), + recorded=recorded_output_lookup( + db, close=True, reference=receipt["reference"] + ), ) _reauthorize_current_target(resolved, scope) identity = { diff --git a/py/api/images.py b/py/api/images.py index f1dfd718..e030c973 100644 --- a/py/api/images.py +++ b/py/api/images.py @@ -61,6 +61,7 @@ try: from ...database.image_favorites import MAX_KEY_LENGTH from ...database.library import display_title from ...database.models import normalize_image_path + from ...database.run_attribution import recorded_output_lookup from ...utils.card_seed import card_seed, header_params, parsed_positive from ...utils.parallel import map_parallel from ...utils.json_strict import dumps_strict @@ -83,6 +84,7 @@ except ImportError: from database.image_favorites import MAX_KEY_LENGTH from database.library import display_title from database.models import normalize_image_path + from database.run_attribution import recorded_output_lookup from utils.card_seed import card_seed, header_params, parsed_positive from utils.parallel import map_parallel from utils.json_strict import dumps_strict @@ -3796,6 +3798,7 @@ class ImageRoutesMixin: image_tags=self._inspection_image_tags( roots[index], Path(relative).parts ), + recorded=recorded_output_lookup(self.db), ) except CurrentFileChanged: return None diff --git a/py/api/inspection_service.py b/py/api/inspection_service.py index 5c8e3e9c..3a99af86 100644 --- a/py/api/inspection_service.py +++ b/py/api/inspection_service.py @@ -3,7 +3,7 @@ import os import stat from pathlib import Path -from typing import Callable +from typing import Any, Callable try: from ...utils.prompt_inspector import ( @@ -100,18 +100,23 @@ def inspect_current_file( saved: dict | None, library: dict, image_tags: Callable[[int, int], dict] | None = None, + recorded: Callable[[Path], Any] | None = None, ) -> dict: """Current-file evidence read between two equal stats of *path*. *image_tags*, when given, is called once with that stable ``(st_size, st_mtime_ns)`` and its facet is returned as ``image_tags`` (spec amendment S1: tags of a byte record verified at exactly that stat). + *recorded*, when given, is asked within the same stat window for what + recorded run history knows about *path* (``RecordedOutput`` or None: the + save node that wrote it and the texts PromptManager recorded, #386). """ before = _current_file_facts(path) try: inspection = read_inspection_file(path) except MetadataReadError: inspection = None + run = recorded(path) if recorded is not None and inspection is not None else None after = _current_file_facts(path) fields = ("st_dev", "st_ino", "st_mode", "st_size", "st_mtime_ns", "st_ctime_ns") if any(getattr(before, key) != getattr(after, key) for key in fields): @@ -129,9 +134,15 @@ def inspect_current_file( profile = ( PREVIEW_INSPECTION_PROFILE if _in_host_temp(path) else INSPECTION_PROFILE ) - # The file name anchors on the saver that wrote it (#381). + # The file name anchors on the saver that wrote it (#381); recorded run + # history names that saver when the file alone cannot (#386). evidence = inspect_prompt_evidence( - inspection.metadata, role_saved, profile=profile, file_name=path.name + inspection.metadata, + role_saved, + profile=profile, + file_name=path.name, + recorded_saver_id=run.saver_id if run is not None else None, + recorded_texts=run.texts if run is not None else None, ) return { "file": { diff --git a/py/api/jobs.py b/py/api/jobs.py index 39b74b36..de92fba8 100644 --- a/py/api/jobs.py +++ b/py/api/jobs.py @@ -47,6 +47,11 @@ import time import uuid from pathlib import PurePosixPath, PureWindowsPath +try: + from ...utils import exit_signal +except ImportError: # tests import the package top-level + from utils import exit_signal + logger = logging.getLogger(__name__) # Start and finish order: wall-clock times can tie (coarse Windows clocks). _sequence = itertools.count() @@ -195,11 +200,21 @@ class Job: self.counters = dict.fromkeys(groups, 0) self.outcomes = {group: [] for group in groups} self.log = [] - self.cancelled = False + self._cancel_requested = False self.last_event = None self.task = None self._subscribers = set() + @property + def cancelled(self): + """Cancel was requested, or the interpreter is exiting (#376): every + check a job already makes between files also stops it at exit.""" + return self._cancel_requested or exit_signal.requested() + + @cancelled.setter + def cancelled(self, value): + self._cancel_requested = bool(value) + @property def record_changing(self): return self.admission == "records" diff --git a/py/api/thumbnail_auto.py b/py/api/thumbnail_auto.py index 0c9095cf..3f425dd9 100644 --- a/py/api/thumbnail_auto.py +++ b/py/api/thumbnail_auto.py @@ -92,6 +92,11 @@ try: except ImportError: # tests import the package top-level from utils.parallel import map_parallel, note_refusal, refused_at_shutdown +try: + from ...utils.exit_signal import ExitRequested +except ImportError: # tests import the package top-level + from utils.exit_signal import ExitRequested + # The placeholder frame a video without a preview is answered with (JPEG # bytes, never written to disk); written previews follow the settings. PLACEHOLDER_SIZE = (thumbnail_settings.DEFAULT_EDGE, thumbnail_settings.DEFAULT_EDGE) @@ -460,6 +465,8 @@ class AutoThumbnailer: def _run(self): try: self._drain() + except ExitRequested: + pass # #376: the interpreter is exiting; queued files are dropped except Exception: self._logger.warning("The automatic preview worker stopped", exc_info=True) finally: diff --git a/py/autotag.py b/py/autotag.py index 8938ed5f..d002f626 100644 --- a/py/autotag.py +++ b/py/autotag.py @@ -7,6 +7,7 @@ Refactored from standalone_tagger.py for integration with PromptManager API. import errno import gc +import inspect import os import sys from pathlib import Path @@ -32,6 +33,54 @@ try: except ImportError: from py.tagging_defaults import DEFAULT_TAG_TEMPLATE +try: + from ..utils import exit_signal +except ImportError: + from utils import exit_signal + + +def abort_hub_downloads() -> None: + """Abort huggingface_hub's Xet transfers (run by the exit hook, #376). + + Xet downloads report progress from native code, which ignores an + exception raised in ``tqdm.update``; huggingface_hub aborts them itself + on Ctrl+C with ``abort_xet_session``. Nothing happens when + huggingface_hub was never imported or has no Xet support. + """ + if "huggingface_hub" not in sys.modules: + return + try: + from huggingface_hub.utils import _xet + except ImportError: + return + abort = getattr(_xet, "abort_xet_session", None) + if callable(abort): + abort() + + +def _stop_at_exit(download) -> Dict[str, Any]: + """Keyword arguments that stop a huggingface_hub download at its next + chunk once the interpreter is exiting (#376), or none when this + huggingface_hub does not take ``tqdm_class``. + + huggingface_hub 1.x removes its partial temp file on any failure; older + versions keep their resumable ``.incomplete`` file. The model's own path + is only written by the final rename either way. + """ + try: + if "tqdm_class" not in inspect.signature(download).parameters: + return {} + from huggingface_hub.utils import tqdm as hub_tqdm + except (ImportError, TypeError, ValueError): + return {} + + class StopAtExit(hub_tqdm): + def update(self, n=1): + exit_signal.checkpoint() + return super().update(n) + + return {"tqdm_class": StopAtExit} + def _autotag_config(): """The saved defaults; imported late, as config needs ComfyUI's server.""" @@ -397,6 +446,7 @@ class AutoTagService: filename=filename, local_dir=str(model_dir), local_dir_use_symlinks=False, + **_stop_at_exit(hf_hub_download), ) if progress_callback: @@ -541,6 +591,13 @@ class AutoTagService: else: return self._download_hf_model(progress_callback) except Exception as e: + if exit_signal.requested(): + # ExitRequested between chunks, or the Xet session aborted + # by the exit hook (abort_hub_downloads). + self.logger.info( + f"Model download stopped: ComfyUI is exiting ({model_type})" + ) + return False self.logger.error(f"Download failed: {e}") if progress_callback: progress_callback(f"Error: {str(e)}", 0) @@ -570,6 +627,7 @@ class AutoTagService: filename=config["filename"], local_dir=str(gguf_dir), local_dir_use_symlinks=False, + **_stop_at_exit(hf_hub_download), ) self.logger.info("GGUF model downloaded") @@ -587,6 +645,7 @@ class AutoTagService: filename=config["mmproj_filename"], local_dir=str(gguf_dir), local_dir_use_symlinks=False, + **_stop_at_exit(hf_hub_download), ) self.logger.info("mmproj downloaded") @@ -613,6 +672,7 @@ class AutoTagService: repo_id=config["repo"], local_dir=str(model_dir), local_dir_use_symlinks=False, + **_stop_at_exit(snapshot_download), ) self.logger.info("HF model downloaded") diff --git a/py/lora_import.py b/py/lora_import.py index 23f053cf..c906eb8d 100644 --- a/py/lora_import.py +++ b/py/lora_import.py @@ -44,8 +44,10 @@ from .lora_manager_layout import ( ) try: + from ..utils import exit_signal from ..utils.hashing import generate_prompt_hash except ImportError: # pragma: no cover - flat test imports + from utils import exit_signal from utils.hashing import generate_prompt_hash LORA_CATEGORY = "lora-manager" @@ -189,14 +191,15 @@ def stage( downloads its CivitAI example images. A sidecar that cannot be read, or whose record fails to build, is skipped by its name, as are the centralized sidecars discovery could not parse. ``cancelled()`` is asked - before each sidecar (raises ImportCancelled); ``on_record(index, + before each sidecar, and so is the exit signal (#376: the legacy scan's + ``cancelled`` does not read it); either raises ImportCancelled. ``on_record(index, record_or_None, staged)`` follows each one. """ sidecars = discovery.sidecars staged = StagedImport(total=len(sidecars)) staged.skipped.extend(discovery.unreadable) for index, sidecar in enumerate(sidecars): - if cancelled(): + if cancelled() or exit_signal.requested(): raise ImportCancelled() record = None path = sidecar.metadata_path diff --git a/tests/browser/legacy-autotag.spec.mjs b/tests/browser/legacy-autotag.spec.mjs index ff96f681..089dafd2 100644 --- a/tests/browser/legacy-autotag.spec.mjs +++ b/tests/browser/legacy-autotag.spec.mjs @@ -1,11 +1,9 @@ -// #328: the legacy admin and gallery auto-tag tools keep working beside System โ€บ Auto-tagging. The model download is -// the `model_download` Activity job; the pages' stream attaches to it. No model is ever downloaded: the fixture's -// fake download waits until the test releases it (`/__test__/tagging/download`). +// #328: the legacy admin and gallery auto-tag batch and review keep working beside System โ€บ Auto-tagging. Since #330 +// the pages download and unload models only through System (legacy-system-links.spec.mjs); a batch never downloads +// one. The fixture's fake download (`/__test__/tagging/download`) is held so nothing is ever fetched. import { startCoverage, saveCoverage } from '../../tools/vendor/browser-coverage.mjs'; import { test, expect } from './fixtures.mjs'; -const TITLE = 'Tagging model download ยท WD14 ViT'; - async function post(request, url, data) { await expect(await request.post(url, data === undefined ? {} : { data })).toBeOK(); } @@ -33,61 +31,6 @@ test.afterEach(async ({ page, request, browserName }) => { await post(request, '/__test__/jobs/restart'); }); -async function startDownload(page, request) { - await page.locator('#autoTagBtn').click(); - const status = page.locator('#wd14VitModelStatus'); - await status.getByRole('button', { name: 'Download', exact: true }).click(); - const modal = page.locator('#autoTagDownloadModal'); - await expect(modal).toBeVisible(); - await expect(page.locator('#downloadStatus')).toHaveText('Downloadingโ€ฆ'); - await expect(page.locator('#downloadProgressBar')).toHaveClass(/animate-pulse/); - await expect(modal).toContainText('The download continues in Activity.'); - await expect(page.locator('#cancelDownloadBtn')).toHaveText('Close'); - const running = (await downloads(request)).filter(job => job.state === 'running'); - expect(running.map(job => [job.title, job.cancellable])).toEqual([[TITLE, false]]); - return { modal, status }; -} - -test('admin download modal closes on complete and Activity lists the job without Cancel', async ({ page, request }) => { - await page.goto('/prompt_manager/admin'); - const { modal, status } = await startDownload(page, request); - - await page.goto('/prompt_manager/workspace#/activity'); - const item = page.locator('#workspace-main section.surface > .activity-item') - .filter({ has: page.getByRole('link', { name: new RegExp(`${TITLE}$`) }) }); - await expect(item.locator('.chip')).toHaveText('Running'); - await expect(item.locator('progress')).not.toHaveAttribute('value'); - await expect(item.getByRole('button', { name: 'Cancel job', exact: true })).toHaveCount(0); - - // Back on the admin page a second download of the same model attaches to the running job. - await page.goto('/prompt_manager/admin'); - await startDownload(page, request); - await post(request, '/__test__/tagging/download', { mode: 'succeed' }); - await expect(modal).toBeHidden(); - await expect(status).toContainText('Downloaded'); - expect((await downloads(request)).map(job => [job.title, job.state])).toEqual([[TITLE, 'finished']]); -}); - -test('gallery download modal closes on complete', async ({ page, request }) => { - await page.goto('/prompt_manager/gallery'); - const { modal, status } = await startDownload(page, request); - await post(request, '/__test__/tagging/download', { mode: 'succeed' }); - await expect(modal).toBeHidden(); - await expect(page.locator('#autoTagModal')).toBeVisible(); - await expect(status).toContainText('Downloaded'); - expect((await downloads(request)).map(job => job.state)).toEqual(['finished']); -}); - -test('a failed download closes the modal and says so', async ({ page, request }) => { - await page.goto('/prompt_manager/gallery'); - const { modal, status } = await startDownload(page, request); - await post(request, '/__test__/tagging/download', { mode: 'fail' }); - await expect(modal).toBeHidden(); - await expect(page.getByText('Download failed: Download failed')).toBeVisible(); - await expect(status.getByRole('button', { name: 'Download', exact: true })).toBeVisible(); - expect((await downloads(request)).map(job => job.state)).toEqual(['failed']); -}); - // (wd14-vit is not downloaded in these tests: beforeEach marks it so the download tests can offer it.) // The gallery's batch reader keyed on `data.status` values the server never sends, so its modal never moved or // closed (#328 Problem 5). It now reads `type` frames as the admin page does. @@ -171,25 +114,6 @@ test('admin review opens the first image', async ({ page }) => { await expect(page.getByText('Failed to start review mode')).toHaveCount(0); }); -test('gallery unload works and is refused while the gate is held', async ({ page, request }) => { - await page.goto('/prompt_manager/gallery'); - await runGalleryBatch(page); // keeps the model in memory - await page.locator('#autoTagBtn').click(); - const unload = page.locator('#unloadModelBtn'); - await expect(unload).toBeVisible(); - - // A held download owns the model gate: Unload is refused with the server's error. - const held = await request.post('/prompt_manager/jobs/model-download', { data: { model_type: 'gguf' } }); - expect(held.status()).toBe(202); - await unload.click(); - await expect(page.getByText(/^Failed to unload model: A tagging model is busy/)).toBeVisible(); - - await post(request, '/__test__/tagging/download', { mode: 'succeed' }); - await expect.poll(async () => (await downloads(request)).map(job => job.state)).toEqual(['finished']); - await unload.click(); - await expect(page.getByText('WD14-SWINV2 model unloaded successfully')).toBeVisible(); -}); - /** The admin review lists every row, videos too: Skip any image that is not one until the confirmation shows. */ async function untilAutoTagged(page, confirm) { // Each image's skip check answers before its confirmation or its error shows. diff --git a/tests/browser/legacy-system-links.spec.mjs b/tests/browser/legacy-system-links.spec.mjs index 50b1c420..37fe9e9e 100644 --- a/tests/browser/legacy-system-links.spec.mjs +++ b/tests/browser/legacy-system-links.spec.mjs @@ -15,9 +15,40 @@ const LABELS = Object.freeze({ diagnostics: 'Diagnostics & logs', }); +async function openPageOptions(page) { + await page.locator('#pageOptionsBtn').click(); + await expect(page.locator('#settingsModal')).toBeVisible(); +} + +/** The auto-tag modal with its model rows read from /autotag/models. */ +async function openAutoTag(page) { + const read = page.waitForResponse(item => item.url().endsWith('/prompt_manager/autotag/models')); + await page.locator('#autoTagBtn').click(); + await read; + await expect(page.locator('#autoTagModal')).toBeVisible(); +} + +const notDownloaded = (page, request) => post(request, '/__test__/tagging/models', { unavailable: ['gguf'] }); +const modelLoaded = (page, request) => post(request, '/__test__/tagging/loaded', { model: 'wd14-vit' }); + // One row per System link of the design's ยง2 map: the page, the control's label, how to find it, the section it opens // and what a test must arrange before the control shows. const LINKS = Object.freeze([ + { page: 'admin', label: 'Settings', locate: page => page.locator('#settingsBtn'), section: 'overview' }, + { page: 'admin', label: 'Maintenance', locate: page => page.locator('#maintenanceBtn'), section: 'maintenance' }, + { page: 'admin', label: 'Backup DB', locate: page => page.locator('#backupBtn'), section: 'backups' }, + { page: 'admin', label: 'Restore DB', locate: page => page.locator('#restoreBtn'), section: 'backups' }, + { page: 'gallery', label: 'Settings', locate: page => page.locator('#settingsBtn'), section: 'appearance' }, + { page: 'admin', label: 'Manage in System (model row)', arrange: notDownloaded, reveal: openAutoTag, + locate: page => page.locator('#ggufModelStatus a'), section: 'tagging' }, + { page: 'gallery', label: 'Manage in System (model row)', arrange: notDownloaded, reveal: openAutoTag, + locate: page => page.locator('#ggufModelStatus a'), section: 'tagging' }, + { page: 'gallery', label: 'Manage in System (Unload Model)', arrange: modelLoaded, reveal: openAutoTag, + locate: page => page.locator('#unloadModelBtn'), section: 'tagging' }, + { page: 'gallery', label: 'Page options Maintenance', reveal: openPageOptions, + locate: page => page.locator('#pageOptionsMaintenanceLink'), section: 'maintenance' }, + { page: 'gallery', label: 'Page options Sources & scans', reveal: openPageOptions, + locate: page => page.locator('#pageOptionsSourcesLink'), section: 'sources' }, { page: 'admin', label: 'Diagnostics', locate: page => page.locator('#diagnosticsBtn'), section: 'diagnostics' }, { page: 'admin', label: 'View Logs', locate: page => page.locator('#logsBtn'), section: 'diagnostics' }, ]); @@ -54,17 +85,31 @@ async function openLink(page, request, link) { return control; } +// Each row opens its section in a new tab and sends no request from the legacy page (the design's `a System link sends +// no request from the legacy page`, checked per row so one slow page never times out the whole table). for (const link of LINKS) { test(`legacy ${link.page} ${link.label} opens System ${link.section}`, async ({ page, context, request }) => { + // The gallery's thumbnail startup check (K9) reads its status on a timer of its own; it is off here. + await page.addInitScript(() => { + localStorage.setItem('gallerySettings', JSON.stringify({ checkThumbnailsAtStartup: false })); + }); const control = await openLink(page, request, link); const legacyUrl = page.url(); await expect(control).toHaveAttribute('href', `workspace#/system/${link.section}`); await expect(control).toHaveAttribute('target', '_blank'); await expect(control).toHaveAttribute('rel', 'noopener'); + // The page's own start-up reads (list, film strips, settings) finish first; then the click must add none. + // Lazy thumbnails may still load whenever the layout changes; they are not the click's. + await page.waitForLoadState('networkidle'); + const sent = []; + const record = item => { if (item.resourceType() !== 'image') sent.push(`${item.method()} ${item.url()}`); }; + page.on('request', record); const opened = context.waitForEvent('page'); await control.click(); const system = await opened; await expect(system).toHaveURL(new RegExp(`/prompt_manager/workspace#/system/${link.section}$`)); + page.off('request', record); + expect(sent, 'a System link sends no request from the legacy page').toEqual([]); // System's page heading is "System"; the open section is the pressed button of its section switcher. const main = system.locator('#workspace-main'); await expect(main.getByRole('heading', { level: 1, name: 'System', exact: true })).toBeVisible(); @@ -75,24 +120,6 @@ for (const link of LINKS) { }); } -test('a System link sends no request from the legacy page', async ({ page, context, request }) => { - for (const link of LINKS) { - const control = await openLink(page, request, link); - // The page's own start-up reads (list, film strips, settings) finish first; then the click must add none. - await page.waitForLoadState('networkidle'); - const sent = []; - const record = item => sent.push(`${item.method()} ${item.url()}`); - page.on('request', record); - const opened = context.waitForEvent('page'); - await control.click(); - const system = await opened; - await system.waitForLoadState('domcontentloaded'); - page.off('request', record); - expect(sent, `${link.page} ${link.label}`).toEqual([]); - await system.close(); - } -}); - test('kept admin flows still work', async ({ page, context }) => { await page.goto(PAGES.admin); const texts = page.locator('#resultsList .prompt-text'); @@ -161,3 +188,99 @@ test('kept admin flows still work', async ({ page, context }) => { await tab.close(); } }); + +test('Page options keeps the page-only settings admin', async ({ page, request }) => { + const before = (await (await request.get('/prompt_manager/settings')).json()).settings; + await page.goto(PAGES.admin); + await expect(page.locator('#resultsList .prompt-text').first()).toBeVisible(); + await page.locator('#pageOptionsBtn').click(); + const dialog = page.locator('#settingsModal'); + await expect(dialog).toBeVisible(); + await expect(dialog.locator('h3')).toHaveText('Page options'); + // Exactly the page-only setting (G5): the list's Infinite Scrolling default. + expect(await dialog.locator('input, select, textarea').evaluateAll(fields => fields.map(field => field.id))) + .toEqual(['infiniteScrollDefault']); + await expect(page.locator('#infiniteScrollDefault')).not.toBeChecked(); + await page.locator('#infiniteScrollDefault').check(); + const saved = page.waitForRequest(item => item.method() === 'POST' && item.url().endsWith('/prompt_manager/settings')); + await page.locator('#saveSettings').click(); + expect((await saved).postDataJSON()).toEqual({ infinite_scroll: true }); + await expect(dialog).toBeHidden(); + // The list header follows the saved default at once; nothing else changed. + await expect(page.locator('#infiniteScrollSession')).toBeChecked(); + const after = (await (await request.get('/prompt_manager/settings')).json()).settings; + expect(after).toEqual({ ...before, infinite_scroll: true }); + await page.reload(); + // The dialog shows the saved default once the page has read it (the header follows it first). + await expect(page.locator('#infiniteScrollSession')).toBeChecked(); + await page.locator('#pageOptionsBtn').click(); + await expect(page.locator('#infiniteScrollDefault')).toBeChecked(); +}); + +test('Page options keeps the page-only settings gallery', async ({ page }) => { + // Native Appearance keys and the startup prompt's own keys are in the shared object before the page opens. + const stored = { imageQuality: 'high', showImageInfo: false, videoMute: false, + autoLoadMetadata: false, workspaceLayout: 'masonry', thumbnailPromptDismissed: 'sig', thumbnailsGenerated: true, + checkThumbnailsAtStartup: false, defaultViewMode: 'list' }; + await page.addInitScript(value => { + if (!sessionStorage.getItem('seeded')) { + sessionStorage.setItem('seeded', '1'); + localStorage.setItem('gallerySettings', value); + } + }, JSON.stringify(stored)); + const listed = page.waitForResponse(item => item.url().includes('/prompt_manager/images/output?')); + await page.goto(PAGES.gallery); + await listed; + // With no Page options control, a stored default view still opens the page in it. + await expect(page.locator('#listViewBtn')).toHaveClass(/bg-pm-accent/); + await openPageOptions(page); + const dialog = page.locator('#settingsModal'); + await expect(dialog.locator('h3')).toHaveText('Page options'); + // Exactly the page-only settings (G5); the default view is System โ€บ Appearance's Default layout. + expect(await dialog.locator('input, select, textarea').evaluateAll(fields => fields.map(field => field.id))).toEqual([ + 'checkThumbnailsAtStartup', 'lazyLoadingToggle', 'defaultLimitSelect', 'gridColumnsSelect', + 'infiniteScrollToggle', 'debugModeToggle', 'apiTimeoutInput']); + await expect(dialog.getByText('Default View Mode')).toHaveCount(0); + await page.locator('#defaultLimitSelect').selectOption('50'); + await page.locator('#debugModeToggle').check(); + await page.locator('#saveSettingsBtn').click(); + await expect(dialog).toBeHidden(); + const read = () => page.evaluate(() => JSON.parse(localStorage.getItem('gallerySettings'))); + let saved = await read(); + expect(saved).toMatchObject({ ...stored, defaultLimit: 50, debugMode: true }); + + // Reset puts back only the page options' defaults; the Appearance, startup-prompt and default-view keys stay. + await openPageOptions(page); + page.once('dialog', dialog => dialog.accept()); + await page.locator('#resetSettingsBtn').click(); + await expect(page.getByText('Page options reset to defaults')).toBeVisible(); + await expect(page.locator('#defaultLimitSelect')).toHaveValue('100'); + saved = await read(); + expect(saved).toMatchObject({ defaultViewMode: 'list', imageQuality: 'high', showImageInfo: false, videoMute: false, + autoLoadMetadata: false, workspaceLayout: 'masonry', thumbnailPromptDismissed: 'sig', thumbnailsGenerated: true, + defaultLimit: 100, debugMode: false, checkThumbnailsAtStartup: true, lazyLoading: true, gridColumns: 8, + infiniteScroll: false, apiTimeout: 30 }); +}); + +test('the legacy auto-tag modal stays open after Manage in System', async ({ page, context, request }) => { + for (const legacy of ['admin', 'gallery']) { + await post(request, '/__test__/tagging/models', { unavailable: ['gguf'] }); + await page.goto(PAGES[legacy]); + await openAutoTag(page); + const link = page.locator('#ggufModelStatus').getByRole('link', { name: 'Manage in System', exact: true }); + await expect(link).toHaveAttribute('title', 'Opens System โ€บ Auto-tagging in a new tab'); + // A downloaded model keeps its status text. + await expect(page.locator('#wd14VitModelStatus')).toHaveText('\u2713 Downloaded'); + const opened = context.waitForEvent('page'); + await link.click(); + const system = await opened; + await expect(system).toHaveURL(/#\/system\/tagging$/); + await expect(page.locator('#autoTagModal')).toBeVisible(); + await system.close(); + // Downloaded in System meanwhile: reopening the modal reads the models again (CUJ 4). + await post(request, '/__test__/tagging/models', { unavailable: [] }); + await page.goto(PAGES[legacy]); + await openAutoTag(page); + await expect(page.locator('#ggufModelStatus')).toHaveText('\u2713 Downloaded'); + } +}); diff --git a/tests/browser/workflows.spec.mjs b/tests/browser/workflows.spec.mjs index e822d998..849839b1 100644 --- a/tests/browser/workflows.spec.mjs +++ b/tests/browser/workflows.spec.mjs @@ -104,33 +104,6 @@ for (const leave of ['beforeunload', 'pagehide']) { }); } -// #222: the gallery's Clear cache clears the server's thumbnails by its own request and keeps this browser's -// preferences and other tools' storage; nothing of Prompt Manager's is cached in a browser, and the toast says so. -test('the gallery cache clear keeps preferences and other tools caches', async ({ page }) => { - await page.addInitScript(() => { - if (sessionStorage.getItem('seeded')) return; - sessionStorage.setItem('seeded', '1'); - localStorage.setItem('pm_workspace_theme', 'light'); - localStorage.setItem('other-tool-setting', 'keep'); - localStorage.setItem('pm_other_tool', 'keep'); - }); - await page.goto('/prompt_manager/gallery'); - await expect(page.locator('#galleryGrid .image-item')).toHaveCount(2); - await page.evaluate(async () => { await (await caches.open('other-tool-gallery-thumbnails')).put('/x', new Response('x')); }); - const clears = []; - page.on('request', req => { if (req.url().includes('/clear-thumbnails')) clears.push(req.method()); }); - page.once('dialog', dialog => dialog.accept()); - await page.locator('#settingsBtn').click(); - await page.locator('#clearCacheBtn').click(); - await expect(page.getByText("Thumbnails cleared; original images untouched. Nothing of Prompt Manager's is cached in this browser.")) - .toBeVisible(); - expect(clears).toEqual(['POST']); - expect(await page.evaluate(async () => ({ - theme: localStorage.getItem('pm_workspace_theme'), other: localStorage.getItem('other-tool-setting'), - prefixed: localStorage.getItem('pm_other_tool'), notice: localStorage.getItem('pm_update_notice_dismissed'), - caches: await caches.keys(), - }))).toEqual({ theme: 'light', other: 'keep', prefixed: 'keep', notice: '3.2.0', caches: ['other-tool-gallery-thumbnails'] }); -}); // #379: the reviewed tags are that image's own tags; no prompt changes. test('actual gallery review generates and persists tags for the selected root\'s image', async ({ page, request }) => { @@ -226,7 +199,7 @@ test('gallery infinite scroll appends a real backend page while retaining existi await expect(await request.post('/__test__/reset', { data: { extra_images: 108 } })).toBeOK(); await page.goto('/prompt_manager/gallery'); await expect(page.locator('#galleryGrid .image-item')).toHaveCount(100); - await page.locator('#settingsBtn').click(); + await page.locator('#pageOptionsBtn').click(); await page.locator('#infiniteScrollToggle').check(); await page.locator('#saveSettingsBtn').click(); await expect(page.locator('#settingsModal')).toBeHidden(); @@ -246,37 +219,6 @@ async function promptSnapshot(request) { })).sort((a, b) => a.id - b.id); } -async function downloadBackup(page) { - const downloading = page.waitForEvent('download'); - await page.locator('#backupBtn').click(); - const download = await downloading; - expect(download.suggestedFilename()).toMatch(/^prompts_backup_.*\.db$/); - expect(await download.failure()).toBeNull(); - const chunks = []; - for await (const chunk of await download.createReadStream()) chunks.push(chunk); - const buffer = Buffer.concat(chunks); - expect(buffer.subarray(0, 16).toString()).toBe('SQLite format 3\0'); - return { name: download.suggestedFilename(), mimeType: 'application/octet-stream', buffer }; -} - -async function chooseRestore(page, file) { - await page.locator('#restoreBtn').click(); - await expect(page.locator('#restoreModal')).toBeVisible(); - await page.locator('#restoreFileInput').setInputFiles(file); -} - -// Confirms the chosen restore and waits for the server's answer, not for a timer: a restore is disk-bound and can take -// longer than an expect timeout on a slow runner (#359). The modal closes only after that answer. -async function confirmRestoreAndWait(page) { - const restored = page.waitForResponse(response => response.url().endsWith('/prompt_manager/restore')); - await page.locator('#confirmRestore').click(); - const response = await restored; - const body = await response.text(); - expect(response.status(), body).toBe(200); - expect(JSON.parse(body)).toMatchObject({ success: true, prompt_count: 2, backup_created: expect.any(String) }); - await expect(page.locator('#restoreModal')).toBeHidden(); -} - function confirmNextDialog(page, accept, message) { page.once('dialog', async dialog => { expect(dialog.type()).toBe('confirm'); @@ -292,88 +234,6 @@ async function deletePromptButton(page, text) { return page.locator(`[data-action="delete"][data-prompt-id="${id}"]`); } -test('downloaded backup restores deleted prompts, tags, and image links through the actual upload form', async ({ page, request }) => { - await page.goto('/prompt_manager/admin'); - await expect(page.locator('.prompt-text')).toHaveCount(2); - const before = await promptSnapshot(request); - expect(before.map(prompt => prompt.tags)).toEqual([['cats'], ['dogs']]); - expect(before.every(prompt => prompt.images.length === 1)).toBe(true); - const backup = await downloadBackup(page); - const button = await deletePromptButton(page, 'Cat portrait'); - confirmNextDialog(page, true, 'delete this prompt'); - await button.click(); - await expect(page.locator('.prompt-text')).toHaveCount(1); - await expect(await request.post('/prompt_manager/save', { data: { text: 'Added after backup', tags: ['temporary'] } })).toBeOK(); - expect(await promptSnapshot(request)).not.toEqual(before); - await chooseRestore(page, backup); - confirmNextDialog(page, true, backup.name); - await confirmRestoreAndWait(page); - await expect(page.locator('.prompt-text')).toHaveCount(2); - await page.reload(); - await expect(page.locator('.prompt-text').filter({ hasText: 'Cat portrait' })).toBeVisible(); - await expect(page.locator('.prompt-text').filter({ hasText: 'Added after backup' })).toHaveCount(0); - expect(await promptSnapshot(request)).toEqual(before); - await page.goto('/prompt_manager/admin#/tags'); - await page.locator('[data-tag-name="cats"]').click(); - await expect(page.locator('#tagPromptsGrid')).toContainText('Cat portrait'); - await page.locator('#tagPromptsGrid img').first().click(); - await expect(page.locator('.viewer-canvas img')).toBeVisible(); - await expect.poll(() => page.locator('.viewer-canvas img').evaluate(image => image.complete && image.naturalWidth > 0)).toBe(true); -}); - -test('restore validation and cancellation preserve live data and allow a valid retry', async ({ page, request }) => { - await page.goto('/prompt_manager/admin'); - const before = await promptSnapshot(request); - const backup = await downloadBackup(page); - let uploads = 0; - page.on('request', request => { if (request.url().endsWith('/prompt_manager/restore')) uploads++; }); - await page.locator('#restoreBtn').click(); - await page.locator('#confirmRestore').click(); - await expect(page.getByText('Please select a database file to restore', { exact: true })).toBeVisible(); - await page.locator('#restoreFileInput').setInputFiles({ ...backup, name: 'backup.txt' }); - await page.locator('#confirmRestore').click(); - await expect(page.getByText('Please select a valid .db file', { exact: true })).toBeVisible(); - expect(uploads).toBe(0); - await page.locator('#restoreFileInput').setInputFiles(backup); - confirmNextDialog(page, false, 'replace your current database'); - await page.locator('#confirmRestore').click(); - await expect(page.locator('#restoreModal')).toBeVisible(); - expect(uploads).toBe(0); - await page.locator('#restoreFileInput').setInputFiles({ name: 'corrupt.db', mimeType: 'application/octet-stream', buffer: Buffer.from('not a SQLite database') }); - confirmNextDialog(page, true, 'corrupt.db'); - const rejected = page.waitForResponse(response => response.url().endsWith('/prompt_manager/restore')); - await page.locator('#confirmRestore').click(); - expect((await rejected).status()).toBe(400); - await expect(page.getByText(/Restore failed:/)).toBeVisible(); - await expect(page.locator('#restoreModal')).toBeVisible(); - await expect(page.locator('#confirmRestore')).toBeEnabled(); - expect(await promptSnapshot(request)).toEqual(before); - await page.locator('#restoreFileInput').setInputFiles(backup); - confirmNextDialog(page, true, backup.name); - await confirmRestoreAndWait(page); - expect(await promptSnapshot(request)).toEqual(before); -}); - -test('backup and restore recover from interrupted network requests without losing data', async ({ page, request }) => { - await page.goto('/prompt_manager/admin'); - const before = await promptSnapshot(request); - await page.route('**/prompt_manager/backup', route => route.abort('failed'), { times: 1 }); - await page.locator('#backupBtn').click(); - await expect(page.getByText(/Backup failed:/)).toBeVisible(); - const backup = await downloadBackup(page); - await chooseRestore(page, backup); - await page.route('**/prompt_manager/restore', route => route.abort('failed'), { times: 1 }); - confirmNextDialog(page, true, backup.name); - await page.locator('#confirmRestore').click(); - await expect(page.getByText(/Restore failed:/)).toBeVisible(); - await expect(page.locator('#confirmRestore')).toBeEnabled(); - await expect(page.locator('#restoreModal')).toBeVisible(); - expect(await promptSnapshot(request)).toEqual(before); - confirmNextDialog(page, true, backup.name); - await confirmRestoreAndWait(page); - expect(await promptSnapshot(request)).toEqual(before); -}); - test('prompt deletion requires confirmation and can retry after a network failure', async ({ page, request }) => { await page.goto('/prompt_manager/admin'); const before = await promptSnapshot(request); @@ -401,98 +261,3 @@ test('prompt deletion requires confirmation and can retry after a network failur await expect(await request.get(url)).toBeOK(); } }); - -// The duplicate scan reads the output root only, but the fixture's input root holds a different -// shared/same.png. A bare /images/serve/ URL tries every gallery root in turn, so once the -// output copy is deleted it answers 200 with the input copy: check each file in its own root. -const serveIn = (root, path) => `/prompt_manager/images/serve/${path.split(/[\\/]/).map(encodeURIComponent).join('/')}?root=${root}`; -const rootOf = url => new URL(url, 'http://gallery.test').searchParams.get('root'); - -// Which copy the scan keeps depends on file times: the fixture stamps shared/same.png with the -// wall clock, and the kernel can stamp its bulk copies a fraction of a millisecond earlier. -// listOrder rewrites the finished scan's groups so a test can pick the order it needs. -async function openDuplicateScan(page, request, listOrder = null) { - await expect(await request.post('/__test__/reset', { data: { extra_images: 2 } })).toBeOK(); - if (listOrder) { - await page.route('**/prompt_manager/scan_duplicates/stream', async route => { - const response = await route.fetch(); - const body = (await response.text()).replace(/^data: (.*)$/gm, (line, json) => { - const event = JSON.parse(json); - if (event.type !== 'complete') return line; - for (const group of event.duplicates) group.images = listOrder(group.images); - return `data: ${JSON.stringify(event)}`; - }); - await route.fulfill({ response, body }); - }); - } - await page.goto('/prompt_manager/gallery'); - await expect(page.locator('#galleryGrid .image-item')).toHaveCount(4); - const originals = await page.locator('#galleryGrid img[data-original]').evaluateAll(images => images.map(image => image.dataset.original)); - const output = rootOf(originals.find(url => url.includes('/bulk/'))); - const input = rootOf(originals.find(url => url.includes('/shared/same.png') && rootOf(url) !== output)); - expect(output).not.toBeNull(); - expect(input).not.toBeNull(); - await page.locator('#settingsBtn').click(); - await page.locator('#scanDuplicatesBtn').click(); - await expect(page.locator('#duplicatesCount')).toHaveText('1'); - const candidates = page.locator('.duplicate-checkbox'); - await expect(candidates).toHaveCount(2); - return { originals, output, input, candidates }; -} - -async function expectOnlySelectedGone(page, request, { originals, output, input }, selected, retained) { - expect((await request.get(serveIn(output, selected))).status()).toBe(404); - await expect(await request.get(serveIn(output, retained))).toBeOK(); - await expect(await request.get(serveIn(input, 'shared/same.png'))).toBeOK(); - await page.reload(); - await expect(page.locator('#galleryGrid .image-item')).toHaveCount(3); - const remaining = await page.locator('#galleryGrid img[data-original]').evaluateAll(images => images.map(image => image.dataset.original)); - expect(remaining).toHaveLength(originals.length - 1); - for (const url of remaining) { - expect(originals).toContain(url); - await expect(await request.get(url)).toBeOK(); - } -} - -test('duplicate cleanup cancels safely and deletes only the selected file after a retry', async ({ page, request }) => { - const scan = await openDuplicateScan(page, request); - const { output, candidates } = scan; - const selected = await candidates.first().getAttribute('data-image-path'); - const retained = await candidates.last().getAttribute('data-image-path'); - await candidates.first().check(); - let deletes = 0; - page.on('request', request => { if (request.url().endsWith('/delete_duplicate_images')) deletes++; }); - confirmNextDialog(page, false, 'permanently delete 1 duplicate image files'); - await page.locator('#removeDuplicatesBtn').click(); - expect(deletes).toBe(0); - await expect(await request.get(serveIn(output, selected))).toBeOK(); - await page.route('**/prompt_manager/delete_duplicate_images', route => route.abort('failed'), { times: 1 }); - confirmNextDialog(page, true, 'permanently delete 1 duplicate image files'); - await page.locator('#removeDuplicatesBtn').click(); - await expect(page.getByText('Failed to delete duplicate images', { exact: true })).toBeVisible(); - await expect(page.locator('#removeDuplicatesBtn')).toBeEnabled(); - await expect(await request.get(serveIn(output, selected))).toBeOK(); - confirmNextDialog(page, true, 'permanently delete 1 duplicate image files'); - const deleted = page.waitForResponse(response => response.url().endsWith('/delete_duplicate_images')); - await page.locator('#removeDuplicatesBtn').click(); - expect((await (await deleted).json())).toMatchObject({ success: true, deleted_count: 1, failed_count: 0 }); - await expect(page.locator('#duplicatesModal')).toBeHidden(); - await expectOnlySelectedGone(page, request, scan, selected, retained); -}); - -test('deleting a duplicate whose path another gallery root also holds removes only the scanned copy', async ({ page, request }) => { - // The order CI saw: a bulk copy stamped first, so shared/same.png is the first Delete candidate. - const ciOrder = ['bulk/0000.png', 'shared/same.png', 'bulk/0001.png']; - const rank = image => ciOrder.indexOf(image.path.replace(/\\/g, '/')); - const scan = await openDuplicateScan(page, request, images => [...images].sort((a, b) => rank(a) - rank(b))); - const selected = await scan.candidates.first().getAttribute('data-image-path'); - const retained = await scan.candidates.last().getAttribute('data-image-path'); - expect(selected.replace(/\\/g, '/')).toBe('shared/same.png'); - await scan.candidates.first().check(); - confirmNextDialog(page, true, 'permanently delete 1 duplicate image files'); - const deleted = page.waitForResponse(response => response.url().endsWith('/delete_duplicate_images')); - await page.locator('#removeDuplicatesBtn').click(); - expect((await (await deleted).json())).toMatchObject({ success: true, deleted_count: 1, failed_count: 0 }); - await expect(page.locator('#duplicatesModal')).toBeHidden(); - await expectOnlySelectedGone(page, request, scan, selected, retained); -}); diff --git a/tests/browser/workspace-system-backups.spec.mjs b/tests/browser/workspace-system-backups.spec.mjs index 6251d0f1..edc369d1 100644 --- a/tests/browser/workspace-system-backups.spec.mjs +++ b/tests/browser/workspace-system-backups.spec.mjs @@ -395,14 +395,15 @@ test('a failed settings read leaves the policy disabled with the reason', async await expect(row.locator('input')).toBeDisabled(); }); -test('the legacy admin Backup and Restore keep working', async ({ page, request }) => { +test('the legacy Backup and Restore routes keep working and the admin page links to System', async ({ page, request }) => { const bytes = await backupBytes(request); const restored = await request.post('/prompt_manager/restore', { multipart: { database_file: { name: 'prompts_backup.db', mimeType: 'application/octet-stream', buffer: bytes } } }); await expect(restored).toBeOK(); expect((await restored.json()).success).toBe(true); + // #330: the admin page's Backup DB and Restore DB open System โ€บ Backup & export. await page.goto('/prompt_manager/admin'); - await expect(page.locator('#restoreFileInput')).toHaveCount(1); + for (const id of ['#backupBtn', '#restoreBtn']) await expect(page.locator(id)).toHaveAttribute('href', 'workspace#/system/backups'); }); /** Start a fake running job (a metadata scan another tab started), which an exclusive restore must wait for. */ diff --git a/tests/browser/workspace_tagging_fixture.py b/tests/browser/workspace_tagging_fixture.py index 1a6801c7..e1250763 100644 --- a/tests/browser/workspace_tagging_fixture.py +++ b/tests/browser/workspace_tagging_fixture.py @@ -248,6 +248,22 @@ def register_tagging_commands( model.unavailable = set(data["unavailable"]) return web.json_response({"success": True}) + @routes.post("/__test__/tagging/loaded") + async def loaded(request: web.Request) -> web.Response: + """Mark one fake model loaded (``{"model": ""}``), as a batch that + kept it in memory leaves it; ``/autotag/models`` then reports + ``model_loaded``. Reset unloads it.""" + data = await request.json() if request.can_read_body else None + if ( + request.query + or not isinstance(data, dict) + or set(data) != {"model"} + or data["model"] not in MODELS + ): + raise web.HTTPBadRequest() + model.loaded = data["model"] + return web.json_response({"success": True}) + @routes.post("/__test__/tagging/download") async def download_mode(request: web.Request) -> web.Response: """Switch the model's download to a fake (never a real download). diff --git a/tests/js/gallery-settings.test.js b/tests/js/gallery-settings.test.js index a8fbf658..7f85fd9a 100644 --- a/tests/js/gallery-settings.test.js +++ b/tests/js/gallery-settings.test.js @@ -76,35 +76,101 @@ test("the workspace keys have their own defaults and validation", () => { assert.equal(normalizeGallerySettings({ defaultViewMode: "masonry" }).defaultViewMode, "grid"); }); -test("a legacy gallery Save keeps the workspace keys and every other stored key", () => { +// #330 S13: the legacy gallery's Page options edit only the page-only keys (PAGE_OPTION_KEYS); System โ€บ Appearance owns the rest, so a Save +// or a Reset from the legacy page keeps every other stored key. +function legacyGallery(stored, controls, extra = {}) { const fs = require("node:fs"); const path = require("node:path"); const vm = require("node:vm"); const settings = require("../../web/js/gallery-settings.js"); - const storage = new Map([["gallerySettings", JSON.stringify({ workspaceLayout: "masonry", workspaceDensity: "compact", someOtherTool: 1 })]]); - const controls = { - lazyLoadingToggle: { checked: true }, imageQualitySelect: { value: "high" }, defaultViewModeSelect: { value: "list" }, - defaultLimitSelect: { value: "200" }, gridColumnsSelect: { value: "6" }, showImageInfoToggle: { checked: false }, - autoLoadMetadataToggle: { checked: true }, cacheMetadataToggle: { checked: true }, showFilePathsToggle: { checked: false }, - sidebarCollapsedByDefaultToggle: { checked: true }, infiniteScrollToggle: { checked: false }, debugModeToggle: { checked: false }, - apiTimeoutInput: { value: "30" }, checkThumbnailsAtStartup: { checked: true }, videoAutoplayToggle: { checked: true }, - videoMuteToggle: { checked: false }, videoLoopToggle: { checked: true }, - }; + const storage = new Map([["gallerySettings", JSON.stringify(stored)]]); const context = vm.createContext({ document: { addEventListener() {}, getElementById: id => controls[id] }, window: {}, console: { debug() {}, warn() {}, error() {} }, GallerySettings: settings, - localStorage: { getItem: key => storage.get(key) ?? null, setItem: (key, value) => storage.set(key, value) }, + localStorage: { getItem: key => storage.get(key) ?? null, setItem: (key, value) => storage.set(key, value), + removeItem: key => storage.delete(key) }, + ...extra, }); const file = path.join(__dirname, "../../web/js/gallery.js"); const Manager = vm.runInContext(`${fs.readFileSync(file, "utf8")}\nGalleryManager;`, context, { filename: file }); const manager = Object.create(Manager.prototype); - Object.assign(manager, { showNotification() {}, hideSettings() {}, applySettings() {} }); + const calls = []; + Object.assign(manager, { showNotification: (message, type) => calls.push([type, message]), hideSettings() {}, + applySettings: applied => calls.push(["apply", applied.lazyLoading]) }); + return { manager, storage, calls, saved: () => JSON.parse(storage.get("gallerySettings")) }; +} + +const PAGE_OPTION_CONTROLS = () => ({ + checkThumbnailsAtStartup: { checked: false }, lazyLoadingToggle: { checked: false }, + defaultLimitSelect: { value: "200" }, + gridColumnsSelect: { value: "6" }, infiniteScrollToggle: { checked: true }, debugModeToggle: { checked: true }, + apiTimeoutInput: { value: "45" }, +}); + +test("a legacy gallery Save keeps the workspace keys and every other stored key", () => { + const { manager, saved } = legacyGallery({ workspaceLayout: "masonry", workspaceDensity: "compact", someOtherTool: 1, + imageQuality: "high", showImageInfo: false, videoMute: false, thumbnailPromptDismissed: "sig", defaultViewMode: "list" }, + PAGE_OPTION_CONTROLS()); manager.saveSettings(); - const saved = JSON.parse(storage.get("gallerySettings")); - assert.equal(saved.workspaceLayout, "masonry"); - assert.equal(saved.workspaceDensity, "compact"); - assert.equal(saved.someOtherTool, 1); - assert.equal(saved.defaultViewMode, "list"); - assert.equal(saved.showImageInfo, false); - assert.equal(saved.videoMute, false); + const result = saved(); + assert.equal(result.workspaceLayout, "masonry"); + assert.equal(result.workspaceDensity, "compact"); + assert.equal(result.someOtherTool, 1); + assert.equal(result.imageQuality, "high"); + assert.equal(result.showImageInfo, false); + assert.equal(result.videoMute, false); + assert.equal(result.thumbnailPromptDismissed, "sig"); + // Page options no longer edit the default view (S13 G5), so a stored one stays. + assert.equal(result.defaultViewMode, "list"); + // The page-only keys come from the dialog. + assert.equal(result.lazyLoading, false); + assert.equal(result.debugMode, true); + assert.equal(result.checkThumbnailsAtStartup, false); + assert.equal(result.defaultLimit, 200); + assert.equal(result.gridColumns, 6); + assert.equal(result.infiniteScroll, true); + assert.equal(result.apiTimeout, 45); +}); + +test("a legacy gallery Reset restores only the page options and keeps every other stored key", () => { + const controls = PAGE_OPTION_CONTROLS(); + const { manager, saved, calls } = legacyGallery({ workspaceLayout: "masonry", imageQuality: "high", videoMute: false, + thumbnailPromptDismissed: "sig", thumbnailsGenerated: true, lazyLoading: false, debugMode: true, defaultLimit: 200, + gridColumns: 6, infiniteScroll: true, apiTimeout: 45, checkThumbnailsAtStartup: false, defaultViewMode: "list" }, controls, + { confirm: message => (calls.push(["confirm", message]), true) }); + manager.resetSettings(); + const result = saved(); + const { DEFAULT_SETTINGS } = require("../../web/js/gallery-settings.js"); + for (const key of ["checkThumbnailsAtStartup", "lazyLoading", "defaultLimit", "gridColumns", + "infiniteScroll", "debugMode", "apiTimeout"]) { + assert.equal(result[key], DEFAULT_SETTINGS[key], key); + } + // The default view is not a page option (S13 G5): Reset keeps a stored one. + assert.equal(result.defaultViewMode, "list"); + assert.equal(result.workspaceLayout, "masonry"); + assert.equal(result.imageQuality, "high"); + assert.equal(result.videoMute, false); + assert.equal(result.thumbnailPromptDismissed, "sig"); + assert.equal(result.thumbnailsGenerated, true); + // The dialog shows the defaults again and the page applies them. + assert.equal(controls.lazyLoadingToggle.checked, DEFAULT_SETTINGS.lazyLoading); + assert.equal(controls.apiTimeoutInput.value, DEFAULT_SETTINGS.apiTimeout); + assert.deepEqual(calls, [["confirm", "Reset these page options to their defaults?"], ["apply", DEFAULT_SETTINGS.lazyLoading], + ["success", "Page options reset to defaults"]]); +}); + +test("Page options no longer hold the default view; System โ€บ Appearance's Default layout is the one control", () => { + const { manager } = legacyGallery({}, {}); + assert.deepEqual([...manager.constructor.PAGE_OPTION_KEYS], ["checkThumbnailsAtStartup", "lazyLoading", "defaultLimit", + "gridColumns", "infiniteScroll", "debugMode", "apiTimeout"]); +}); + +test("a viewer mute or loop click remembers only that key and never reads the Page options dialog", () => { + const { manager, saved, calls } = legacyGallery({ defaultLimit: 200, lazyLoading: false, videoMute: true }, {}); + manager.storeSetting("videoMute", false); + const result = saved(); + assert.equal(result.videoMute, false); + assert.equal(result.defaultLimit, 200); + assert.equal(result.lazyLoading, false); + assert.deepEqual(calls, []); }); diff --git a/tests/js/gallery-thumbnails.test.js b/tests/js/gallery-thumbnails.test.js index cb135161..118306a2 100644 --- a/tests/js/gallery-thumbnails.test.js +++ b/tests/js/gallery-thumbnails.test.js @@ -112,11 +112,12 @@ test('saving gallery settings retains the remembered thumbnail dismissal', () => }); // #249 System S5: the stream's work is now an Activity job that outlives the page's connection, so the gallery's -// Cancel asks the server to stop the job. The stream's first frame, `job {id, started}`, names the job; the gallery +// Cancel asks the server to stop the job. Since #330 the startup dialog is the gallery's only thumbnail stream (System +// โ€บ Sources & scans generates the rest). The stream's first frame, `job {id, started}`, names the job; the gallery // cancels by that id, only a job this stream started, and a Cancel before the id waits for it (review I1). const JOB = 'b'.repeat(32); -function streamFixture({ modal = false, postFails = false } = {}) { +function streamFixture({ postFails = false } = {}) { const posts = []; const warnings = []; const gets = []; @@ -146,18 +147,20 @@ function streamFixture({ modal = false, postFails = false } = {}) { return { success: true }; } } }); const emit = (type, data) => (listeners[type] || []).forEach(handler => handler({ data: JSON.stringify(data) })); + // The dialog's Cancel runs the handler and from then on reads as cancelled, as the startup dialog does. + let cancelled = false; const ui = { addStatusMessage() {}, progressText: element(), progressPercent: element(), progressBar: element(), - progressDetails: element(), progressETA: element(), isCancelled: () => false, - onCancel: handler => { ui.cancel = handler; } }; - const result = modal ? manager.generateThumbnailsWithProgressInModal(ui) - : manager.generateThumbnailsWithProgress({ quality: 'medium' }); + progressDetails: element(), progressETA: element(), isCancelled: () => cancelled, + onCancel: handler => { ui.cancel = () => { cancelled = true; handler(); }; } }; + const result = manager.generateThumbnailsWithProgressInModal(ui); const outcome = result.then(value => ({ value }), error => ({ error: error.message })); - const cancel = () => (modal ? ui.cancel() : elements.cancelThumbnailsBtn.click()); + const cancel = () => ui.cancel(); const settle = () => new Promise(resolve => setImmediate(resolve)); return { emit, source, posts, gets, warnings, outcome, cancel, settle }; } const cancelUrl = id => `/prompt_manager/jobs/${id}/cancel`; +const CANCELLED = 'Thumbnail generation cancelled; previews already written are kept'; test('Cancel before the job id arrives cancels that job once it does', async () => { const f = streamFixture(); @@ -165,8 +168,8 @@ test('Cancel before the job id arrives cancels that job once it does', async () await f.settle(); assert.deepEqual(f.posts, []); assert.equal(f.source.closed, 0); // kept open until the stream names its job - assert.deepEqual(await f.outcome, { error: 'Thumbnail generation cancelled by user' }); - f.emit('status', { phase: 'scanning', message: 'Scanningโ€ฆ' }); + assert.deepEqual(await f.outcome, { error: CANCELLED }); + // The job frame is the stream's first (the dialog closes the stream on any later frame once cancelled). f.emit('job', { id: JOB, started: true }); await f.settle(); assert.equal(JSON.stringify(f.posts), JSON.stringify([[cancelUrl(JOB), {}]])); @@ -185,7 +188,7 @@ test('Cancel after the id cancels it at once, and a double Cancel posts once', a await f.settle(); assert.equal(JSON.stringify(f.posts), JSON.stringify([[cancelUrl(JOB), {}]])); assert.equal(f.source.closed, 1); - assert.deepEqual(await f.outcome, { error: 'Thumbnail generation cancelled by user' }); + assert.deepEqual(await f.outcome, { error: CANCELLED }); assert.deepEqual(f.gets, []); }); @@ -216,11 +219,11 @@ test('a cancel post that fails is reported, not thrown', async () => { await f.settle(); assert.equal(f.posts.length, 1); assert.equal(f.warnings.length, 1); - assert.deepEqual(await f.outcome, { error: 'Thumbnail generation cancelled by user' }); + assert.deepEqual(await f.outcome, { error: CANCELLED }); }); test('the startup dialog cancels its own job by id too', async () => { - const f = streamFixture({ modal: true }); + const f = streamFixture(); f.cancel(); await f.settle(); assert.deepEqual(f.posts, []); diff --git a/tests/js/legacy-refusals.test.js b/tests/js/legacy-refusals.test.js index c06daedc..78d15c9b 100644 --- a/tests/js/legacy-refusals.test.js +++ b/tests/js/legacy-refusals.test.js @@ -65,109 +65,12 @@ function load(file, className, fetchUrl, extra = {}, dom = elements()) { const BUSY = 'Scan for metadata already running'; -test('gallery cache clear reports a refused server clear and never claims success', async () => { - const calls = []; - const { page, dom, notifications, storage } = load('gallery.js', 'GalleryManager', async url => { - calls.push(url); - return jsonResponse(409, { success: false, error: BUSY }); - }); - page.getSettings = () => ({ thumbnailsGenerated: true }); - dom.getElementById('thumbnailStatus').textContent = 'Generated'; - - await page.clearCache(); - - assert.deepEqual(calls, ['/prompt_manager/images/clear-thumbnails']); - assert.deepEqual(notifications, [['error', `Thumbnails were not cleared: ${BUSY}`]]); - assert.equal(dom.nodes.thumbnailStatus.textContent, 'Generated'); - assert.equal(storage.get('gallerySettings'), undefined); // thumbnailsGenerated is not reset -}); - -test('gallery cache clear says what it cleared and keeps preferences and other data (#222)', async () => { - const calls = []; - const { page, dom, notifications, storage } = load('gallery.js', 'GalleryManager', async url => { - calls.push(url); - return jsonResponse(200, { success: true, cleared_count: 3 }); - }); - page.getSettings = () => ({ thumbnailsGenerated: true }); - - await page.clearCache(); - - // The server's thumbnails are cleared by their own explicit call; nothing Prompt Manager owns is cached in this - // browser, so no browser entry is removed and the toast says exactly that. - assert.deepEqual(calls, ['/prompt_manager/images/clear-thumbnails']); - assert.deepEqual(notifications, [['success', - "Thumbnails cleared; original images untouched. Nothing of Prompt Manager's is cached in this browser."]]); - assert.equal(dom.nodes.thumbnailStatus.textContent, 'Not generated'); - assert.equal(JSON.parse(storage.get('gallerySettings')).thumbnailsGenerated, false); - assert.equal(storage.get('userData'), 'keep'); - assert.equal(storage.get('pm_cached'), '1'); - assert.equal(storage.get('pm_workspace_theme'), 'light'); - assert.equal(storage.get('pm_update_notice_dismissed'), '3.3.0'); -}); - -test("gallery cache clear never deletes another tool's gallery or thumbnail caches (#222)", async () => { - const names = new Set(['comfy-gallery', 'thumbnail-cache']); - const caches = { keys: async () => [...names], delete: async name => names.delete(name) }; - const { page, notifications } = load('gallery.js', 'GalleryManager', - async () => jsonResponse(200, { success: true }), { caches, window: { caches } }); - page.getSettings = () => ({}); - - await page.clearCache(); - - assert.deepEqual([...names], ['comfy-gallery', 'thumbnail-cache']); - assert.equal(notifications.at(-1)[0], 'success'); -}); - -test('gallery cache clear reports a lost server clear instead of success', async () => { - const { page, dom, notifications, storage } = load('gallery.js', 'GalleryManager', async () => { - throw new TypeError('Failed to fetch'); - }); - page.getSettings = () => ({ thumbnailsGenerated: true }); - dom.getElementById('thumbnailStatus').textContent = 'Generated'; - - await page.clearCache(); - - assert.deepEqual(notifications, [['error', 'Thumbnails were not cleared: the server could not be reached.']]); - assert.equal(dom.nodes.thumbnailStatus.textContent, 'Generated'); - assert.equal(storage.get('gallerySettings'), undefined); -}); - -test('gallery cache clear says so when browser storage is denied', async () => { - const denied = { get length() { throw new Error('SecurityError'); }, key() { throw new Error('SecurityError'); }, - getItem() { throw new Error('SecurityError'); }, setItem() { throw new Error('SecurityError'); }, - removeItem() { throw new Error('SecurityError'); } }; - const { page, notifications } = load('gallery.js', 'GalleryManager', - async () => jsonResponse(200, { success: true }), { localStorage: denied }); - page.getSettings = () => ({}); - - await page.clearCache(); - - assert.deepEqual(notifications, [['warning', - 'Thumbnails cleared; original images untouched. This browser refused access to its storage, so nothing there was cleared.']]); -}); - -test('admin LoRA import shows a refusal frame and closes its modal', async () => { - const { page, dom, timers, notifications } = load('admin.js', 'PromptAdmin', - async () => sseResponse([{ type: 'error', status: BUSY, progress: 0, message: BUSY }])); - page.saveLoraSettings = async () => true; - page.loadStatistics = () => assert.fail('a refused import must not reload statistics'); - - await page.runLoraImport(); - - assert.equal(dom.nodes.loraImportStatus.textContent, `Error: ${BUSY}`); - assert.deepEqual(notifications.filter(([type]) => type === 'error'), [['error', `LoRA import failed: ${BUSY}`]]); - assert.equal(timers.length, 1); - timers[0](); - assert.deepEqual(notifications.at(-1), ['hide', 'loraImportModal']); -}); - test('admin settings save shows why config.json refused it', async () => { const message = 'config.json cannot be read, so nothing was saved. Fix or remove the file, then save again.'; const { page, notifications } = load('admin.js', 'PromptAdmin', async () => jsonResponse(409, { success: false, persisted: false, code: 'config-unreadable', error: message })); page.settings = {}; - page._collectScanPaths = () => []; - page.saveLoraSettings = async () => assert.fail('nothing else is saved after a refusal'); + page.setInfiniteScroll = () => assert.fail('nothing changes after a refusal'); await page.saveSettings(); @@ -178,27 +81,30 @@ test('admin settings save keeps its plain message when the reply has no reason', const { page, notifications } = load('admin.js', 'PromptAdmin', async () => ({ ok: false, status: 500, json: async () => { throw new SyntaxError('not json'); } })); page.settings = {}; - page._collectScanPaths = () => []; await page.saveSettings(); assert.deepEqual(notifications, [['error', 'Failed to save settings']]); }); -test('admin maintenance names the job that refused it', async () => { - const { page, dom, notifications } = load('admin.js', 'PromptAdmin', - async () => jsonResponse(409, { success: false, error: BUSY }), { - escapeHtml: text => String(text), - document: { readyState: 'loading', addEventListener() {}, getElementById: id => dom.getElementById(id), - querySelectorAll: selector => (selector === '.maintenance-option:checked' ? [{ id: 'vacuum' }] : []) }, - }); +// #330 S13-9: the admin page's reduced Page options save sends only the list's scroll default, so a stale legacy form +// can no longer overwrite scan folders or other settings edited in System. +test('admin Page options save posts only infinite_scroll', async () => { + const sent = []; + const { page, dom, notifications } = load('admin.js', 'PromptAdmin', async (url, init) => { + sent.push([url, init.method, JSON.parse(init.body)]); + return jsonResponse(200, { success: true, restart_required: true }); + }); + page.settings = { infiniteScroll: false }; + const scrolled = []; + page.setInfiniteScroll = (on, options) => scrolled.push([on, { ...options }]); + dom.getElementById('infiniteScrollDefault').checked = true; - await page.runMaintenance(); + await page.saveSettings(); - const results = dom.nodes.maintenanceResults.innerHTML; - assert.match(results, /Maintenance Failed/); - assert.match(results, new RegExp(BUSY)); - assert.doesNotMatch(results, /HTTP 409/); - assert.deepEqual(notifications, [['error', 'โŒ Maintenance failed']]); - assert.equal(dom.nodes.runMaintenanceBtn.disabled, false); + assert.deepEqual(sent, [['/prompt_manager/settings', 'POST', { infinite_scroll: true }]]); + assert.deepEqual(scrolled, [[true, { session: false }]]); + assert.equal(page.settings.infiniteScroll, true); + assert.deepEqual(notifications, [['success', 'Page options saved'], ['hide', 'settingsModal']]); }); + diff --git a/tests/js/workspace-file-inspector.test.js b/tests/js/workspace-file-inspector.test.js index 904b7d14..f74621b1 100644 --- a/tests/js/workspace-file-inspector.test.js +++ b/tests/js/workspace-file-inspector.test.js @@ -185,6 +185,26 @@ test('legacy fill applies to local records', async () => { assert.notEqual(zeroed.prompts.positive.text, ''); }); +test('a branch named by recorded run history shows the Recorded run badge', async () => { + // #386: the server labels sides whose save node recorded history named; Execution recorded stays stronger. + const base = await localRecord(new File([fs.readFileSync(new URL('f09.png', PARITY))], 'f09.png')); + const recorded = {...base, prompts:{positive:{...base.prompts.positive, source:'recorded-run'}, negative:{...base.prompts.negative, source:'recorded-run'}}}; + let view = renderFull(recorded); + try { + assert.deepEqual(view.texts(n => n.className === 'chip blue'), ['Recorded run']); + assert.ok(view.texts(n => n.tagName === 'p').includes('Prompt evidence: recorded run')); + } finally { view.done(); } + view = renderFull({...recorded, prompts:{...recorded.prompts, negative:{...recorded.prompts.negative, source:'execution'}}}); + 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. function tagged(imageTags, library = {status:'linked', tags:['Minimal', 'warm_light', 'editorial'], tags_scope:'positive'}) { return {...record(), imageTags, library}; diff --git a/tests/test_api_backup_restore.py b/tests/test_api_backup_restore.py index e9b451cf..32da612d 100644 --- a/tests/test_api_backup_restore.py +++ b/tests/test_api_backup_restore.py @@ -264,6 +264,41 @@ class TestRestoreRoute(BackupRestoreTestCase): texts = sorted(p["text"] for p in self.api.db.search_prompts()) self.assertEqual(texts, ["restored a", "restored b"]) + async def test_restore_round_trip_keeps_tags_and_image_links(self): + """A downloaded backup brings back every prompt with its tags and images. + + The legacy admin page's Restore form left for System (#330); this keeps + its browser test's data check at the route. + """ + db = self.api.db + originals = {} + for text, tags, image in ( + ("round trip cat", ["cats", "pets"], "cat.png"), + ("round trip dog", ["dogs"], "dog.png"), + ): + prompt_id = db.save_prompt( + text=text, tags=tags, prompt_hash=generate_prompt_hash(text) + ) + path = self._write(image, b"\x89PNG\r\n\x1a\n") + self.assertTrue(db.link_image_to_prompt(str(prompt_id), path)) + originals[text] = (prompt_id, sorted(tags), [path]) + + resp = await self.client.get("/prompt_manager/backup") + self.assertEqual(resp.status, 200) + backup = self._write("downloaded.db", await resp.read()) + self.assertTrue(db.delete_prompt(originals["round trip dog"][0])) + + resp = await self._restore(backup) + self.assertEqual(resp.status, 200, await resp.text()) + + restored = {p["text"]: p for p in self.api.db.search_prompts()} + self.assertEqual(sorted(restored), sorted(originals)) + for text, (prompt_id, tags, paths) in originals.items(): + self.assertEqual(restored[text]["id"], prompt_id) + self.assertEqual(sorted(restored[text]["tags"]), tags) + images = self.api.db.get_prompt_images(str(prompt_id)) + self.assertEqual([image["image_path"] for image in images], paths) + async def test_restore_leaves_no_temp_files_behind(self): upload = self._make_db_file(["restored"]) created = [] diff --git a/tests/test_api_history_outputs.py b/tests/test_api_history_outputs.py index c250cc27..c43f43f1 100644 --- a/tests/test_api_history_outputs.py +++ b/tests/test_api_history_outputs.py @@ -21,6 +21,8 @@ from tests.test_api_images import ImageAPITestCase, make_png from tests.test_api_prompt_inspector import graph, png from tests.test_execution_context_database import bundle as context_bundle from tests.test_execution_history_database import JOB, SESSION +from tests.saver_anchor_graphs import BRANCH_TEXTS, shared_prefix +from tests.test_recorded_saver_anchor import LLM_TEXT, text_generate_run from utils.execution_contexts import ContextBundle from utils.execution_history_events import make_finish, make_start from utils.execution_outputs import OutputBundle, OutputReference, snapshot_output_roots @@ -1505,3 +1507,88 @@ class HistoryOutputAPITests(ImageAPITestCase): body = await response.json() self.assertEqual(response.status, 200, body) self.assertEqual(body["prompts"]["positive"]["text"], "Readable portrait") + + async def recorded_shared_prefix_run(self): + """#386: the dataset shape, branch 3's saver wrote the file in a run.""" + path = png(self.output_dir / "IMG_GEN_00371_.png", prompt=shared_prefix()) + # make_start/make_finish observe the run at 0 ms: the file was written then. + os.utime(path, ns=(1_000_000, 1_000_000)) + start = self.capture( + [self.ref(node="33", filename=path.name), self.ref("43", 1, "other.png")] + ) + return start + + async def test_both_inspect_routes_name_the_branch_recorded_history_saved(self): + start = await self.recorded_shared_prefix_run() + _, body = await self.outputs(start) + descriptor = next( + item["current_candidate"]["media"] + for item in body["items"] + if item["occurrence"]["output_node_id"] == "33" + ) + response = await self.current(descriptor["inspection_url"]) + recorded = await response.json() + self.assertEqual(response.status, 200, recorded) + response = await self.client.get( + "/prompt_manager/images/inspect/IMG_GEN_00371_.png", params={"root": "0"} + ) + images = await response.json() + self.assertEqual(response.status, 200, images) + for result in (recorded, images): + self.assertEqual(result["resolution"]["state"], "resolved") + self.assertEqual( + result["prompts"]["positive"], + { + "status": "resolved", + "text": BRANCH_TEXTS[2], + "source": "recorded-run", + }, + ) + + async def test_a_file_rewritten_after_its_run_keeps_several_prompts(self): + await self.recorded_shared_prefix_run() + os.utime(self.output_dir / "IMG_GEN_00371_.png") # written now: not that run + response = await self.client.get( + "/prompt_manager/images/inspect/IMG_GEN_00371_.png", params={"root": "0"} + ) + body = await response.json() + self.assertEqual(response.status, 200, body) + self.assertEqual(body["resolution"]["state"], "ambiguous") + self.assertEqual(body["prompts"]["positive"]["status"], "ambiguous") + + async def test_a_failing_history_lookup_judges_the_file_alone(self): + await self.recorded_shared_prefix_run() + with ( + patch( + "database.run_attribution.recorded_output", + side_effect=sqlite3.OperationalError("locked"), + ), + self.assertLogs("prompt_manager.run_attribution", "WARNING"), + ): + response = await self.client.get( + "/prompt_manager/images/inspect/IMG_GEN_00371_.png", + params={"root": "0"}, + ) + body = await response.json() + self.assertEqual(response.status, 200, body) + self.assertEqual(body["resolution"]["state"], "ambiguous") + + async def test_a_language_model_text_shows_what_promptmanager_recorded(self): + path = png(self.output_dir / "moss_00026_.png", prompt=text_generate_run()) + os.utime(path, ns=(1_000_000, 1_000_000)) + start = self.capture([self.ref(node="9", filename=path.name)]) + self.api.db.write_execution_contexts( + start.run_id, + start.storage_epoch, + context_bundle("", LLM_TEXT, anchor="7"), + ) + response = await self.client.get( + "/prompt_manager/images/inspect/moss_00026_.png", params={"root": "0"} + ) + body = await response.json() + self.assertEqual(response.status, 200, body) + self.assertEqual( + body["prompts"]["positive"], + {"status": "resolved", "text": LLM_TEXT, "source": "execution"}, + ) + self.assertEqual(body["prompts"]["negative"]["text"], "") diff --git a/tests/test_exit_cancel.py b/tests/test_exit_cancel.py new file mode 100644 index 00000000..e3fb5ba5 --- /dev/null +++ b/tests/test_exit_cancel.py @@ -0,0 +1,463 @@ +"""Work running at Ctrl+C stops at its next safe point (#376). + +ComfyUI catches Ctrl+C in ``main.py`` and returns without closing its event +loop. Python then joins every executor thread (``concurrent.futures``'s +``_python_exit``, a ``threading._register_atexit`` callback) before any +``atexit`` callback runs, so a call still running on the default executor +used to keep the process alive until it returned. The package registers its +own ``threading._register_atexit`` hook, which runs before that join: it sets +the exit signal that ``Job.cancelled`` and ``map_parallel`` read, and kills +the encoder processes. + +The child-process test starts three long calls on the default executor (a +registry job checking ``job.cancelled`` between files, a ``map_parallel`` +batch and an encoder process that ignores the exit, as ffmpeg does on +Windows), returns from its main module and must exit far sooner than any of +them would end. +""" + +import json +import os +import subprocess +import sys +import tempfile +import textwrap +import threading +import unittest +from unittest import mock + +ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +sys.path.insert(0, ROOT) + +try: + from utils import exit_signal # noqa: E402 +except ImportError: # before #376: only the child-process test can run + exit_signal = None + +# The parent never waits longer than this for the child (the hang watchdog +# allows 120 s per test). Each fake job alone would run 300 s or more. +CHILD_CEILING_SECONDS = 60 +# From the child's return to its last atexit callback: the stopped calls end +# at their next file, item or chunk (the encoder is killed). +EXIT_BOUND_SECONDS = 15 +JOB_SECONDS = 600 +# The fake encoder outlives the ceiling, yet a failing run leaks it only +# briefly (the parent cannot reach the child's own child to kill it). +ENCODER_SECONDS = 90 +REPORT_PREFIX = "PM_EXIT_REPORT " + +CHILD = textwrap.dedent(""" + import asyncio + import atexit + import contextlib + import importlib + import io + import json + import os + import sys + import threading + import time + + root, tmp = sys.argv[1], sys.argv[2] + job_seconds, encoder_seconds = float(sys.argv[3]), int(sys.argv[4]) + sys.path.insert(0, root) + state = {"stopped": {}} + + def report(): + returned = state.get("returned") + print("PM_EXIT_REPORT " + json.dumps({ + "exit_seconds": None if returned is None else time.monotonic() - returned, + "stopped": state["stopped"], + })) + sys.stdout.flush() + + # Registered first, so it runs after every other atexit callback, and + # every atexit callback runs after the executor threads were joined. + atexit.register(report) + + from tests.test_init_package import _import_package, fake_server_module + + output = os.path.join(tmp, "output") + with contextlib.redirect_stdout(io.StringIO()): + package, _ = _import_package( + "pm_exit_child", + os.path.join(tmp, "prompts.db"), + output, + fake_server_module(queue_task_done=True), + ) + parallel = importlib.import_module("pm_exit_child.utils.parallel") + encode = importlib.import_module("pm_exit_child.py.api.thumbnail_encode") + registry = package.api._jobs + entered = {name: threading.Event() for name in ("job", "batch", "encoder")} + + def job_work(job): + # A job's thread work: one "file" at a time, checking its cancel + # between files the way the thumbnail and import jobs do. + entered["job"].set() + deadline = time.monotonic() + job_seconds + while time.monotonic() < deadline: + if job.cancelled: + state["stopped"]["job"] = True + return + time.sleep(0.05) + + def batch_item(index): + entered["batch"].set() + time.sleep(0.5) + return index + + def batch_work(): + try: + parallel.map_parallel(batch_item, range(int(job_seconds)), 2) + except Exception as error: + state["stopped"]["batch"] = type(error).__name__ + + def encoder_work(): + # Stands for ffmpeg on Windows, which runs without a console and + # never sees Ctrl+C: only a kill ends it before its timeout. + cmd = [sys.executable, "-c", "import time; time.sleep(%d)" % encoder_seconds] + try: + encode.run_encoder( + cmd, + timeout=encoder_seconds, + track=lambda _process: entered["encoder"].set(), + ) + except Exception as error: + state["stopped"]["encoder"] = type(error).__name__ + + async def begin(): + loop = asyncio.get_running_loop() + + async def runner(job): + await loop.run_in_executor(None, job_work, job) + + registry.start("exit-test", "Exit test", admission="read", runner=runner) + loop.run_in_executor(None, batch_work) + loop.run_in_executor(None, encoder_work) + for name, event in entered.items(): + if not await loop.run_in_executor(None, event.wait, 30): + sys.exit("not started: " + name) + + loop = asyncio.new_event_loop() + asyncio.set_event_loop(loop) + loop.run_until_complete(begin()) + state["returned"] = time.monotonic() + # Return with the three calls running and the loop open, as ComfyUI does + # after Ctrl+C. + """) + + +class TestExitStopsRunningWork(unittest.TestCase): + def test_running_executor_work_stops_at_exit(self): + with tempfile.TemporaryDirectory() as tmp: + os.makedirs(os.path.join(tmp, "output")) + script = os.path.join(tmp, "child.py") + with open(script, "w", encoding="utf-8") as handle: + handle.write(CHILD) + env = dict(os.environ) + env.pop("PROMPT_MANAGER_CONFIG_PATH", None) + env["PROMPT_MANAGER_DATA_DIR"] = tmp + env["PYTHONIOENCODING"] = "utf-8" + try: + completed = subprocess.run( + [ + sys.executable, + script, + ROOT, + tmp, + str(JOB_SECONDS), + str(ENCODER_SECONDS), + ], + cwd=ROOT, + env=env, + capture_output=True, + timeout=CHILD_CEILING_SECONDS, + encoding="utf-8", + errors="replace", + ) + except subprocess.TimeoutExpired as hung: + self.fail( + f"the child did not exit within {CHILD_CEILING_SECONDS} s\n" + f"stdout:\n{hung.stdout}\nstderr:\n{hung.stderr}" + ) + detail = f"stdout:\n{completed.stdout}\nstderr:\n{completed.stderr}" + self.assertEqual(completed.returncode, 0, detail) + reports = [ + line[len(REPORT_PREFIX) :] + for line in completed.stdout.splitlines() + if line.startswith(REPORT_PREFIX) + ] + self.assertEqual(len(reports), 1, detail) + report = json.loads(reports[0]) + self.assertLess(report["exit_seconds"], EXIT_BOUND_SECONDS, detail) + self.assertEqual( + report["stopped"], + {"job": True, "batch": "ExitRequested", "encoder": "EncoderClosed"}, + detail, + ) + for text in ("Traceback", "Background work not stopped in time"): + self.assertNotIn(text, completed.stderr, detail) + + +class _Isolated(unittest.TestCase): + """Each test starts with the signal clear and no hook installed.""" + + def setUp(self): + exit_signal.reset() + self.addCleanup(exit_signal.reset) + + +class TestInstall(_Isolated): + def test_the_hook_is_registered_once_before_the_executor_join(self): + calls = [] + with mock.patch.object( + exit_signal.threading, + "_register_atexit", + side_effect=lambda func: calls.append(func), + create=True, + ): + self.assertTrue(exit_signal.install()) + self.assertTrue(exit_signal.install()) + self.assertEqual(len(calls), 1) + self.assertFalse(exit_signal.requested()) + calls[0]() + self.assertTrue(exit_signal.requested()) + + def test_a_missing_internal_hook_degrades_to_the_old_exit(self): + with mock.patch.object(exit_signal, "threading") as fake: + del fake._register_atexit + self.assertFalse(exit_signal.install()) + self.assertFalse(exit_signal.requested()) + + def test_a_refused_registration_degrades_to_the_old_exit(self): + refuse = RuntimeError("can't register atexit after shutdown") + with mock.patch.object( + exit_signal.threading, "_register_atexit", side_effect=refuse, create=True + ): + self.assertFalse(exit_signal.install()) + + def test_a_failing_exit_action_never_escapes_the_hook(self): + # An exception out of a threading atexit callback would abort + # threading._shutdown and skip the executor join itself. + calls = [] + + def boom(): + raise KeyboardInterrupt # a second Ctrl+C + + with mock.patch.object( + exit_signal.threading, + "_register_atexit", + side_effect=lambda func: calls.append(func), + create=True, + ): + exit_signal.install(boom) + with self.assertLogs(exit_signal.logger, "ERROR"): + calls[0]() + self.assertTrue(exit_signal.requested()) + + +class TestCheckpoints(_Isolated): + def test_checked_runs_until_the_signal_then_raises(self): + seen = [] + record = exit_signal.checked(seen.append) + record(1) + exit_signal.request() + with self.assertRaises(exit_signal.ExitRequested): + record(2) + self.assertEqual(seen, [1]) + + def test_map_parallel_starts_no_item_after_the_signal(self): + from utils.parallel import map_parallel + + started = [] + gate = threading.Event() + + def item(index): + started.append(index) + if index == 0: + exit_signal.request() + gate.set() + return index + + with self.assertRaises(exit_signal.ExitRequested): + map_parallel(item, range(50), 1) + self.assertTrue(gate.is_set()) + self.assertEqual(started, [0]) + + def test_job_cancelled_follows_the_signal_and_its_own_cancel(self): + from py.api.jobs import Job + + job = Job("scan", "Scan", admission="records") + self.assertFalse(job.cancelled) + job.cancelled = True + self.assertTrue(job.cancelled) + other = Job("scan", "Scan", admission="records", cancellable=False) + exit_signal.request() + self.assertTrue(other.cancelled) + + def test_the_scan_walk_stops_between_directories(self): + from py.api.admin import _collect_media_files + + with tempfile.TemporaryDirectory() as tmp: + for name in ("a", "b"): + os.makedirs(os.path.join(tmp, name)) + with open(os.path.join(tmp, name, "x.png"), "wb") as image: + image.write(b"x") + self.assertEqual(len(_collect_media_files([tmp], [".png"])), 2) + exit_signal.request() + with self.assertRaises(exit_signal.ExitRequested): + _collect_media_files([tmp], [".png"]) + + +class TestLoraImportStopsAtExit(_Isolated): + def test_staging_stops_before_the_next_sidecar_whatever_the_caller_checks(self): + import types + + from py import lora_import + + discovery = types.SimpleNamespace( + sidecars=[object(), object()], unreadable=[], layout=None + ) + exit_signal.request() + with tempfile.TemporaryDirectory() as tmp: + with self.assertRaises(lora_import.ImportCancelled): + lora_import.stage( + discovery, + tmp, + media_extensions=frozenset(), + cancelled=lambda: False, # the legacy scan's own check + ) + + +class _FakeTqdm: + """huggingface_hub.utils.tqdm as far as a download uses it.""" + + def __init__(self, *args, **kwargs): + self.n = 0 + + def update(self, n=1): + self.n += n + + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + + +def _fake_hub(chunks_before_exit): + """A ``huggingface_hub`` whose download writes chunks to a temp file and, + like huggingface_hub 1.x, removes it on any failure before the rename.""" + import types + + def hf_hub_download( + repo_id, filename, local_dir=None, local_dir_use_symlinks=None, tqdm_class=None + ): + final = os.path.join(local_dir, filename) + partial = final + ".abc.incomplete" + bar = (tqdm_class or _FakeTqdm)(total=10) + try: + with open(partial, "wb") as handle: + for index in range(10): + if index == chunks_before_exit: + exit_signal.request() + bar.update(1) + handle.write(b"x") + os.replace(partial, final) + finally: + if os.path.exists(partial): + os.unlink(partial) + return final + + hub = types.ModuleType("huggingface_hub") + hub.hf_hub_download = hf_hub_download + hub.snapshot_download = lambda **kwargs: None + utils = types.ModuleType("huggingface_hub.utils") + utils.tqdm = _FakeTqdm + hub.utils = utils + return {"huggingface_hub": hub, "huggingface_hub.utils": utils} + + +class TestModelDownloadStopsAtExit(_Isolated): + def test_a_download_stops_between_chunks_and_leaves_no_file(self): + from py.autotag import AutoTagService + + with tempfile.TemporaryDirectory() as tmp: + service = AutoTagService(models_dir=tmp) + with mock.patch.dict(sys.modules, _fake_hub(chunks_before_exit=3)): + with self.assertLogs(service.logger, "INFO") as logs: + self.assertFalse(service.download_model("wd14-vit")) + folder = os.path.join(tmp, "WD14-ViT") + self.assertEqual(os.listdir(folder), []) + self.assertTrue(any("exiting" in line for line in logs.output), logs.output) + self.assertFalse(any("Download failed" in line for line in logs.output)) + + def test_a_download_without_the_signal_finishes(self): + from py.autotag import AutoTagService + + with tempfile.TemporaryDirectory() as tmp: + service = AutoTagService(models_dir=tmp) + with mock.patch.dict(sys.modules, _fake_hub(chunks_before_exit=None)): + self.assertTrue(service.download_model("wd14-vit")) + folder = os.path.join(tmp, "WD14-ViT") + self.assertEqual( + sorted(os.listdir(folder)), ["model.onnx", "selected_tags.csv"] + ) + + def test_xet_transfers_are_aborted_the_way_hub_aborts_on_ctrl_c(self): + import types + + from py.autotag import abort_hub_downloads + + aborted = [] + xet = types.ModuleType("huggingface_hub.utils._xet") + xet.abort_xet_session = lambda: aborted.append(True) + hub = _fake_hub(chunks_before_exit=None) + hub["huggingface_hub.utils._xet"] = xet + hub["huggingface_hub.utils"]._xet = xet + with mock.patch.dict(sys.modules, hub): + abort_hub_downloads() + self.assertEqual(aborted, [True]) + with mock.patch.dict(sys.modules): + sys.modules.pop("huggingface_hub", None) + abort_hub_downloads() # never imported: nothing to abort + self.assertEqual(aborted, [True]) + + def test_an_aborted_xet_download_is_logged_as_stopped(self): + from py.autotag import AutoTagService + + hub = _fake_hub(chunks_before_exit=None) + + def aborted(*args, **kwargs): + exit_signal.request() # the hook ran, then hub raised + raise RuntimeError("xet session aborted") + + hub["huggingface_hub"].hf_hub_download = aborted + with tempfile.TemporaryDirectory() as tmp: + service = AutoTagService(models_dir=tmp) + with mock.patch.dict(sys.modules, hub): + with self.assertLogs(service.logger, "INFO") as logs: + self.assertFalse(service.download_model("wd14-vit")) + self.assertTrue(any("exiting" in line for line in logs.output), logs.output) + self.assertFalse(any("Download failed" in line for line in logs.output)) + + def test_an_old_hub_without_tqdm_class_downloads_as_before(self): + from py.autotag import AutoTagService + + hub = _fake_hub(chunks_before_exit=None) + full = hub["huggingface_hub"].hf_hub_download + + def old_download( + repo_id, filename, local_dir=None, local_dir_use_symlinks=None + ): + return full(repo_id, filename, local_dir, local_dir_use_symlinks) + + hub["huggingface_hub"].hf_hub_download = old_download + with tempfile.TemporaryDirectory() as tmp: + service = AutoTagService(models_dir=tmp) + with mock.patch.dict(sys.modules, hub): + self.assertTrue(service.download_model("wd14-vit")) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_recorded_saver_anchor.py b/tests/test_recorded_saver_anchor.py new file mode 100644 index 00000000..3ca852c6 --- /dev/null +++ b/tests/test_recorded_saver_anchor.py @@ -0,0 +1,379 @@ +"""Recorded run history names the saver that wrote a file (#386, #240). + +When a file's graph alone cannot name its prompt (sampler branches that differ +only in their prompt and share one saver prefix), the inspector asks recorded +history which save node wrote the file and walks from that node, as the #381 +file-name anchor does. A result the file alone resolves is never changed; a +recorded saver that names nothing certain leaves the file "several possible +prompts". The resolved sides are labelled as coming from the recorded run. +""" + +import json +import unittest + +from utils import prompt_pair_resolver as resolver +from utils.prompt_inspector import inspect_prompt_evidence + +try: + from tests import saver_anchor_graphs as shapes +except ImportError: # discovered with ``-s tests`` + import saver_anchor_graphs as shapes + + +def _resolve(graph, **options): + return resolver.resolve_prompt_pair( + {"prompt": graph}, profile=resolver.INSPECTION_PROFILE, **options + ) + + +def _positive(result): + side = result["positive"] + return side["text"] if isinstance(side, dict) else None + + +def _evidence(graph, **options): + return inspect_prompt_evidence({"prompt": json.dumps(graph)}, **options) + + +# ``branch_bench``'s savers are node ``N3`` of branch N (10s based). +def _saver(index): + return str(index * 10 + 3) + + +def guider_dataset(): + """The dataset run's shape: SamplerCustomAdvanced branches on BasicGuider + (no negative), every branch saved with one prefix.""" + graph = { + "1": {"class_type": "CheckpointLoaderSimple", "inputs": {"ckpt_name": "m"}}, + "2": {"class_type": "EmptyLatentImage", "inputs": {"width": 64}}, + "4": {"class_type": "RandomNoise", "inputs": {"noise_seed": 1}}, + "5": {"class_type": "KSamplerSelect", "inputs": {"sampler_name": "euler"}}, + "6": { + "class_type": "BasicScheduler", + "inputs": {"model": ["1", 0], "steps": 8, "scheduler": "simple"}, + }, + } + for index, text in enumerate(shapes.BRANCH_TEXTS, start=1): + base = index * 10 + graph[str(base)] = { + "class_type": "CLIPTextEncode", + "inputs": {"text": text, "clip": ["1", 1]}, + } + graph[str(base + 4)] = { + "class_type": "BasicGuider", + "inputs": {"model": ["1", 0], "conditioning": [str(base), 0]}, + } + graph[str(base + 1)] = { + "class_type": "SamplerCustomAdvanced", + "inputs": { + "noise": ["4", 0], + "guider": [str(base + 4), 0], + "sampler": ["5", 0], + "sigmas": ["6", 0], + "latent_image": ["2", 0], + }, + } + graph[str(base + 2)] = { + "class_type": "VAEDecode", + "inputs": {"samples": [str(base + 1), 0], "vae": ["1", 2]}, + } + graph[str(base + 3)] = { + "class_type": "SaveImage", + "inputs": { + "filename_prefix": "dataset/IMG_GEN", + "images": [str(base + 2), 0], + }, + } + return graph + + +class TestRecordedSaverInResolver(unittest.TestCase): + def test_the_recorded_saver_picks_its_branch_of_a_shared_prefix(self): + for index, text in enumerate(shapes.BRANCH_TEXTS, start=1): + with self.subTest(branch=index): + result = _resolve( + shapes.shared_prefix(), + file_name="IMG_GEN_00371_.png", + recorded_saver_id=_saver(index), + ) + self.assertEqual(result["state"], "resolved") + self.assertEqual(_positive(result), text) + self.assertEqual(result["sampler_id"], str(index * 10 + 1)) + + def test_without_a_recorded_saver_the_shared_prefix_stays_several(self): + result = _resolve(shapes.shared_prefix(), file_name="IMG_GEN_00371_.png") + self.assertEqual(result["state"], "ambiguous") + + def test_a_recorded_saver_the_graph_lacks_anchors_nothing(self): + for node in ("missing", "3", "2"): + with self.subTest(node=node): + result = _resolve(shapes.shared_prefix(), recorded_saver_id=node) + self.assertEqual(result["state"], "ambiguous") + + def test_a_recorded_saver_of_an_unknown_class_anchors(self): + graph = shapes.branch_bench( + prefixes=["dataset/IMG_GEN"] * len(shapes.BRANCH_TEXTS), + saver="VendorSaveImage", + ) + result = _resolve(graph, recorded_saver_id=_saver(2)) + self.assertEqual(_positive(result), shapes.BRANCH_TEXTS[1]) + + def test_the_default_profile_ignores_a_recorded_saver(self): + result = resolver.resolve_prompt_pair( + {"prompt": shapes.shared_prefix()}, recorded_saver_id=_saver(1) + ) + self.assertEqual(result["state"], "ambiguous") + + +class TestRecordedSaverInInspector(unittest.TestCase): + def test_the_inspector_shows_the_recorded_branch_labelled_as_recorded(self): + evidence = _evidence( + shapes.shared_prefix(), + file_name="IMG_GEN_00371_.png", + recorded_saver_id=_saver(3), + ) + self.assertEqual(evidence["resolution"]["state"], "resolved") + positive = evidence["prompts"]["positive"] + self.assertEqual(positive["text"], shapes.BRANCH_TEXTS[2]) + self.assertEqual(positive["source"], "recorded-run") + self.assertEqual(evidence["prompts"]["negative"]["source"], "recorded-run") + + def test_a_guider_branch_without_a_negative_shows_its_positive(self): + self.assertEqual( + _evidence(guider_dataset(), file_name="IMG_GEN_00371_.png")["resolution"][ + "state" + ], + "ambiguous", + ) + evidence = _evidence( + guider_dataset(), file_name="IMG_GEN_00371_.png", recorded_saver_id="23" + ) + self.assertEqual( + evidence["prompts"]["positive"], + { + "status": "resolved", + "text": shapes.BRANCH_TEXTS[1], + "source": "recorded-run", + }, + ) + 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( + shapes.branch_bench(), + file_name="s45_branch_2_00001_.png", + recorded_saver_id=_saver(4), + ) + positive = evidence["prompts"]["positive"] + self.assertEqual(positive["text"], shapes.BRANCH_TEXTS[1]) + self.assertEqual(positive["source"], "graph-inferred") + + def test_a_recorded_saver_that_names_nothing_keeps_several_prompts(self): + evidence = _evidence(shapes.shared_prefix(), recorded_saver_id="missing") + self.assertEqual(evidence["resolution"]["state"], "ambiguous") + self.assertEqual(evidence["prompts"]["positive"]["status"], "ambiguous") + + def test_no_recorded_saver_is_unchanged(self): + self.assertEqual( + _evidence(shapes.shared_prefix(), recorded_saver_id=None), + _evidence(shapes.shared_prefix()), + ) + + +LLM_TEXT = "a red vintage car parked by the sea, golden hour" + + +def text_generate_run(): + """The maintainer's moss run: a language model writes PromptManager's text.""" + return { + "1": {"class_type": "CheckpointLoaderSimple", "inputs": {"ckpt_name": "m"}}, + "2": {"class_type": "EmptyLatentImage", "inputs": {"width": 64}}, + "17": {"class_type": "PrimitiveStringMultiline", "inputs": {"value": "a car"}}, + "14": { + "class_type": "PrimitiveStringMultiline", + "inputs": {"value": "You are a prompt engineer."}, + }, + "11": { + "class_type": "TextGenerate", + "inputs": { + "prompt": ["17", 0], + "system_prompt": ["14", 0], + "clip": ["1", 1], + "max_length": 4096, + "sampling_mode.seed": 0, + }, + }, + "36": { + "class_type": "PromptManagerPair", + "inputs": { + "text": ["11", 0], + "negative_text": "", + "prepend_text": "", + "append_text": "", + "clip": ["1", 1], + }, + }, + "7": { + "class_type": "KSampler", + "inputs": { + "seed": 1, + "steps": 20, + "cfg": 1.0, + "sampler_name": "euler", + "scheduler": "simple", + "denoise": 1.0, + "model": ["1", 0], + "positive": ["36", 0], + "negative": ["36", 1], + "latent_image": ["2", 0], + }, + }, + "8": { + "class_type": "VAEDecode", + "inputs": {"samples": ["7", 0], "vae": ["1", 2]}, + }, + "9": { + "class_type": "SaveImage", + "inputs": {"filename_prefix": "moss", "images": ["8", 0]}, + }, + } + + +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.""" + + def test_a_run_time_text_shows_what_the_run_recorded(self): + alone = _evidence(text_generate_run(), file_name="moss_00026_.png") + self.assertEqual(alone["resolution"]["reason_code"], "runtime-value") + evidence = _evidence( + text_generate_run(), + file_name="moss_00026_.png", + recorded_saver_id="9", + recorded_texts={"7": {"positive": LLM_TEXT, "negative": ""}}, + ) + self.assertEqual( + evidence["prompts"], + { + "positive": { + "status": "resolved", + "text": LLM_TEXT, + "source": "execution", + }, + "negative": {"status": "resolved", "text": "", "source": "execution"}, + }, + ) + self.assertEqual(evidence["resolution"]["state"], "resolved") + self.assertEqual(evidence["resolution"]["sampler_id"], "7") + + 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) + self.assertEqual(evidence["prompts"]["negative"]["status"], "unresolved") + + def test_texts_that_do_not_name_the_open_sampler_change_nothing(self): + alone = _evidence(text_generate_run()) + for texts in ( + {"8": {"positive": LLM_TEXT, "negative": ""}}, + {"7": {"positive": None, "negative": ""}}, + {"7": {"positive": " ", "negative": ""}}, + {"7": "not roles"}, + {}, + ): + with self.subTest(texts=texts): + self.assertEqual( + _evidence( + text_generate_run(), recorded_saver_id="9", recorded_texts=texts + ), + alone, + ) + + 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__": + unittest.main() diff --git a/tests/test_run_attribution.py b/tests/test_run_attribution.py index ce3c60c3..050d710a 100644 --- a/tests/test_run_attribution.py +++ b/tests/test_run_attribution.py @@ -9,13 +9,22 @@ import unittest from pathlib import Path from unittest import mock -from database.run_attribution import stored_run_prompt +from database.run_attribution import ( + RecordedOutput, + recorded_output, + stored_run_prompt, +) from tests import test_execution_history_database as history_tests from tests.background_stops import close_handler from tests.test_api_admin import AdminAPITestCase -from tests.test_execution_context_database import snapshot +from tests.test_execution_context_database import known, snapshot from tests.test_prompt_attribution import issue_graph, llm_text_graph, make_tracker -from utils.execution_contexts import ContextBundle, RecipeContext +from utils.execution_contexts import ( + ContextBundle, + PromptSnapshot, + RecipeContext, + TextEvidence, +) from utils.execution_outputs import ( OutputBundle, OutputReference, @@ -38,28 +47,34 @@ class RecordedRuns: start = history_tests.HistoryDatabaseTests.start finish = history_tests.HistoryDatabaseTests.finish - def record_run(self, db, output_root, files, positives, item=0, window=None): + def record_run( + self, db, output_root, files, positives, item=0, window=None, nodes=None + ): started, finished = window or (now_ms() - 5000, now_ms() + 5000) start = self.start(db, item, started) db.write_execution_start(start) finish = self.finish(start, time=finished) db.write_execution_finish(finish) - db.write_execution_contexts( - start.run_id, - start.storage_epoch, - ContextBundle( - tuple( - RecipeContext(str(4 + i), snapshot(text), snapshot(""), "inferred") - for i, text in enumerate(positives) + if positives: # a run with no PromptManager node records no contexts + db.write_execution_contexts( + start.run_id, + start.storage_epoch, + ContextBundle( + tuple( + RecipeContext( + str(4 + i), snapshot(text), snapshot(""), "inferred" + ) + for i, text in enumerate(positives) + ), + "complete", + "captured", ), - "complete", - "captured", - ), - ) + ) root = snapshot_output_roots(STYLE, output_root, output_root).output_key + nodes = nodes or ["9"] * len(files) references = tuple( - OutputReference("9", i, "output", sub, name, STYLE, root) - for i, (sub, name) in enumerate(files) + OutputReference(node, i, "output", sub, name, STYLE, root) + for i, (node, (sub, name)) in enumerate(zip(nodes, files)) ) db.write_execution_outputs_for_finish( finish, OutputBundle(references, "complete", "reported") @@ -171,6 +186,186 @@ class TestStoredRunPrompt(RecordedRuns, unittest.TestCase): self.assertEqual(linked, [prompt_id]) +class TestRecordedOutputNode(RecordedRuns, unittest.TestCase): + """#386: the save node history says wrote a file, under the same rules.""" + + close_databases = history_tests.HistoryDatabaseTests.close_databases + database = history_tests.HistoryDatabaseTests.database + setUp = TestStoredRunPrompt.setUp + + def node(self, path=None): + 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") + Path(other).write_bytes(b"png") + self.record_run( + self.db, + self.output, + [("sub", "img_00001_.png"), ("sub", "img_00002_.png")], + [], + nodes=["13", "23"], + ) + self.assertEqual(self.node(), "13") + self.assertEqual(self.node(other), "23") + + def test_a_run_without_prompt_manager_contexts_still_names_the_saver(self): + self.record_run(self.db, self.output, [("sub", "img_00001_.png")], []) + self.assertEqual(self.node(), "9") + + def test_two_runs_reporting_the_file_name_nothing(self): + self.record_run( + self.db, self.output, [("sub", "img_00001_.png")], [], nodes=["13"] + ) + self.record_run( + self.db, self.output, [("sub", "img_00001_.png")], [], item=1, nodes=["23"] + ) + self.assertIsNone(self.node()) + + def test_one_run_reporting_the_file_from_two_savers_names_nothing(self): + self.record_run( + self.db, + self.output, + [("sub", "img_00001_.png"), ("sub", "img_00001_.png")], + [], + nodes=["13", "23"], + ) + self.assertIsNone(self.node()) + + def test_a_file_written_outside_the_run_names_nothing(self): + self.record_run( + self.db, + self.output, + [("sub", "img_00001_.png")], + [], + window=(1_000_000, 2_000_000), + ) + self.assertIsNone(self.node()) + + def test_unrecorded_outside_or_unknown_roots_name_nothing(self): + self.record_run(self.db, self.output, [("sub", "img_00001_.png")], []) + other = os.path.join(self.output, "sub", "other.png") + Path(other).write_bytes(b"png") + self.assertIsNone(self.node(other)) + 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( + self.db, self.output, [("sub", "img_00001_.png")], ["an llm prompt"] + ) + self.assertEqual( + recorded_output(self.db, self.image, self.output), + RecordedOutput("9", {"4": {"positive": "an llm prompt", "negative": ""}}), + ) + + def test_texts_only_inferred_from_the_graph_are_not_recorded(self): + start = self.start(self.db, 0, now_ms() - 5000) + self.db.write_execution_start(start) + finish = self.finish(start, time=now_ms() + 5000) + self.db.write_execution_finish(finish) + inferred = TextEvidence("graph text", "inferred", "graph_literal", None) + self.db.write_execution_contexts( + start.run_id, + start.storage_epoch, + ContextBundle( + ( + RecipeContext( + "7", + PromptSnapshot(inferred, inferred, known(""), known("")), + snapshot("kept"), + "inferred", + ), + ), + "complete", + "captured", + ), + ) + root = snapshot_output_roots(STYLE, self.output, self.output).output_key + self.db.write_execution_outputs_for_finish( + finish, + OutputBundle( + ( + OutputReference( + "9", 0, "output", "sub", "img_00001_.png", STYLE, root + ), + ), + "complete", + "reported", + ), + ) + self.assertEqual( + recorded_output(self.db, self.image, self.output).texts, + {"7": {"positive": None, "negative": "kept"}}, + ) + + def test_several_savers_name_no_saver_but_keep_the_texts(self): + self.record_run( + self.db, + self.output, + [("sub", "img_00001_.png"), ("sub", "img_00001_.png")], + ["text"], + nodes=["13", "23"], + ) + found = recorded_output(self.db, self.image, self.output) + self.assertIsNone(found.saver_id) + self.assertEqual(found.texts, {"4": {"positive": "text", "negative": ""}}) + + def test_the_route_lookup_logs_a_failure_and_names_nothing(self): + from database.run_attribution import recorded_output_lookup + + broken = mock.Mock() + broken.model.get_connection.return_value.execute.side_effect = ( + sqlite3.OperationalError("locked") + ) + broken.get_execution_contexts.side_effect = sqlite3.OperationalError("x") + with mock.patch( + "database.run_attribution.host_output_directory", return_value=self.output + ): + self.assertIsNone(recorded_output_lookup(broken)(self.image)) + with ( + mock.patch( + "database.run_attribution._recorded_runs", + side_effect=sqlite3.OperationalError("locked"), + ), + self.assertLogs("prompt_manager.run_attribution", "WARNING"), + ): + self.assertIsNone( + recorded_output_lookup(broken, close=True)(self.image) + ) + broken.model.close.assert_called_once_with() + + def test_a_history_reference_spells_the_file_as_history_recorded_it(self): + # On Windows the history route's target is normcased (lower case); + # history matches names exactly, so the route passes its reference. + from database.run_attribution import recorded_output_lookup + + upper = os.path.join(self.output, "sub", "IMG_00001_.png") + Path(upper).write_bytes(b"png") + self.record_run(self.db, self.output, [("sub", "IMG_00001_.png")], []) + target = os.path.join(self.output, "sub", "img_00001_.png") + reference = { + "folder_type": "output", + "subfolder": "sub", + "filename": "IMG_00001_.png", + } + with mock.patch( + "database.run_attribution.host_output_directory", return_value=self.output + ): + found = recorded_output_lookup(self.db, reference=reference)(target) + self.assertEqual(found.saver_id, "9") + temp = {**reference, "folder_type": "temp"} + self.assertIsNone(recorded_output_lookup(self.db, reference=temp)(target)) + + def test_a_library_without_history_tables_names_nothing(self): + broken = mock.Mock() + broken.model.get_connection.return_value.execute.side_effect = ( + sqlite3.OperationalError("no such table") + ) + self.assertIsNone(recorded_output(broken, self.image, self.output)) + + class TestScanUsesHistory(RecordedRuns, AdminAPITestCase): """The output scan applies the same rules as the monitor.""" diff --git a/tests/vendor/ci-workflow.test.mjs b/tests/vendor/ci-workflow.test.mjs index cfafcaae..d8ed881a 100644 --- a/tests/vendor/ci-workflow.test.mjs +++ b/tests/vendor/ci-workflow.test.mjs @@ -67,6 +67,45 @@ const browserJobIds = [...allJobs].filter(([, body]) => /npm run --prefix tools\ const mergeJob = () => allJobs.get('frontend-coverage-merge'); const verifyStep = () => steps(mergeJob()).find(step => /name: Verify every shard's raw coverage arrived/.test(step)); +// Jobs that run only in the merge queue, on main and on demand (#420): skipped on pull_request. +const QUEUE_ONLY_IF = /^ {4}if: github\.event_name != 'pull_request'$/m; +const EVENTS = ['pull_request', 'merge_group', 'push', 'workflow_dispatch']; + +function gateVariables(jobText) { + return new Map([...jobText.matchAll(/^ {10}([A-Z_]+): \$\{\{ needs\.([\w-]+)\.result \}\}$/gm)] + .map(match => [match[2], match[1]])); +} + +function runGate(jobText, env) { + const script = runScript(steps(jobText).find(step => /run: \|/.test(step))); + try { + execFileSync('bash', ['--noprofile', '--norc', '-eo', 'pipefail', '-c', script], { env: { PATH: process.env.PATH, ...env }, stdio: 'pipe' }); + return true; + } catch { + return false; + } +} + +// A gate passes only when every always-on job succeeded and every queue-only job is skipped on a +// pull request and succeeded on every other event. Anything else fails it. +function assertGateBehaviour(jobText, queueOnly) { + assert.match(jobText, /^ {10}EVENT_NAME: \$\{\{ github\.event_name \}\}$/m, 'the gate knows the event'); + const variables = gateVariables(jobText); + for (const id of queueOnly) assert.ok(variables.has(id), `${id} reaches the gate`); + for (const event of EVENTS) { + const expected = id => (queueOnly.includes(id) && event === 'pull_request' ? 'skipped' : 'success'); + const green = Object.fromEntries([...variables].map(([id, name]) => [name, expected(id)])); + assert.equal(runGate(jobText, { ...green, EVENT_NAME: event }), true, `${event}: the expected results pass`); + for (const [id, name] of variables) { + for (const result of ['success', 'failure', 'cancelled', 'skipped']) { + if (result === expected(id)) continue; + assert.equal(runGate(jobText, { ...green, EVENT_NAME: event, [name]: result }), false, + `${event}: ${id}=${result} must fail the gate`); + } + } + } +} + test('viewerjs keeps its name and needs every frontend job', () => { const viewerjs = allJobs.get('viewerjs'); assert.ok(viewerjs, 'the required context is the job id viewerjs'); @@ -76,32 +115,17 @@ test('viewerjs keeps its name and needs every frontend job', () => { assert.deepEqual([...needs].sort(), [...others].sort(), 'viewerjs needs every other job in the workflow'); assert.deepEqual([...needs].sort(), ['browser-workflows', 'frontend-coverage', 'frontend-coverage-merge']); assert.match(viewerjs, /^ {4}if: always\(\)$/m, 'viewerjs runs (and fails) when a needed job failed or was skipped'); - const script = runScript(steps(viewerjs).find(step => /run: \|/.test(step))); - for (const id of needs) { - const variable = new RegExp(`^ {10}([A-Z_]+): \\$\\{\\{ needs\\.${id}\\.result \\}\\}$`, 'm').exec(viewerjs); - assert.ok(variable, `${id}'s result reaches the check`); - assert.match(script, new RegExp(`test "\\$${variable[1]}" = success`), `${id} must be exactly success`); - } + assert.deepEqual([...gateVariables(viewerjs).keys()].sort(), [...needs].sort(), 'every needed result reaches the check'); }); -test('viewerjs fails unless every needed job succeeded', () => { - const viewerjs = allJobs.get('viewerjs'); - const script = runScript(steps(viewerjs).find(step => /run: \|/.test(step))); - const variables = [...viewerjs.matchAll(/^ {10}([A-Z_]+): \$\{\{ needs\.[\w-]+\.result \}\}$/gm)].map(match => match[1]); - const run = env => { - try { - execFileSync('bash', ['--noprofile', '--norc', '-eo', 'pipefail', '-c', script], { env: { PATH: process.env.PATH, ...env }, stdio: 'pipe' }); - return true; - } catch { - return false; - } - }; - const allSuccess = Object.fromEntries(variables.map(name => [name, 'success'])); - assert.equal(run(allSuccess), true); - for (const name of variables) { - for (const result of ['failure', 'cancelled', 'skipped']) { - assert.equal(run({ ...allSuccess, [name]: result }), false, `${name}=${result} must fail viewerjs`); - } +test('viewerjs requires the browser shards everywhere but on a pull request', () => { + assertGateBehaviour(allJobs.get('viewerjs'), ['browser-workflows']); +}); + +test('Firefox/WebKit page workflows run only outside pull requests; coverage runs on every event', () => { + assert.match(allJobs.get('browser-workflows'), QUEUE_ONLY_IF); + for (const id of ['frontend-coverage', 'frontend-coverage-merge']) { + assert.doesNotMatch(allJobs.get(id), /^ {4}if:/m, `${id} runs on every event`); } }); @@ -224,7 +248,8 @@ test('browser job timeouts keep a real margin over measured runs', () => { }); // Every job is bounded (#350): a hang ends at a measured limit instead of GitHub's 6-hour default. -const BOUNDED_WORKFLOWS = ['test.yml', 'code-quality.yml', 'viewerjs-check.yml']; +const GATING_WORKFLOWS = ['test.yml', 'code-quality.yml', 'viewerjs-check.yml']; +const BOUNDED_WORKFLOWS = [...GATING_WORKFLOWS, 'portability.yml']; const readWorkflow = name => readFileSync(new URL(`../../.github/workflows/${name}`, import.meta.url), 'utf8'); test('every job in the CI workflows has a numeric timeout-minutes', () => { @@ -276,3 +301,73 @@ test('the bandit pre-commit hook fails at the severity CI enforces and passes ev assert.doesNotMatch(hook, /true|exit-zero|bash -c/, 'nothing masks the exit code or drops a file'); assert.doesNotMatch(hook, /pass_filenames: false/, 'the hook scans the staged files'); }); + +// Event split (#420): pull requests run a fast subset, the merge queue and main run everything. +const testJobs = () => jobs(readWorkflow('test.yml')); + +test('the Python matrix is Linux 3.12 on a pull request and Linux 3.12-3.14 on every other event', () => { + const body = testJobs().get('test'); + assert.deepEqual(matrixList(body, 'os'), ['ubuntu-latest'], 'Windows and macOS never gate a merge'); + const match = /^ {8}python-version: \$\{\{ github\.event_name == 'pull_request' && fromJSON\('(\[[^\]]*\])'\) \|\| fromJSON\('(\[[^\]]*\])'\) \}\}$/m.exec(body); + assert.ok(match, 'python-version is chosen by event'); + assert.deepEqual({ pr: JSON.parse(match[1]), other: JSON.parse(match[2]) }, { pr: ['3.12'], other: ['3.12', '3.13', '3.14'] }); + assert.match(body, /^ {6}fail-fast: false$/m); + assert.match(body, /^ {4}name: py\$\{\{ matrix\.python-version \}\} on \$\{\{ matrix\.os \}\}$/m); +}); + +test('Windows and macOS run 3.12-3.14 on main, nightly and on demand, never on a PR or in the queue', () => { + const text = readWorkflow('portability.yml'); + const triggers = text.slice(text.indexOf('\non:'), text.indexOf('\nconcurrency:')); + for (const event of ['push', 'schedule', 'workflow_dispatch']) assert.match(triggers, new RegExp(`^ {2}${event}:`, 'm')); + for (const event of ['pull_request', 'merge_group']) assert.doesNotMatch(triggers, new RegExp(`^ {2}${event}`, 'm')); + assert.match(triggers, /^ {4}branches: \[main\]$/m, 'push runs only for main'); + const body = jobs(text).get('test'); + assert.deepEqual(matrixList(body, 'os'), ['windows-latest', 'macos-latest']); + assert.deepEqual(matrixList(body, 'python-version'), ['"3.12"', '"3.13"', '"3.14"']); + assert.match(body, /^ {6}fail-fast: false$/m); + assert.match(body, /^ {8}run: python -m tests\.run_suite -v$/m); + assert.doesNotMatch(text, /cancel-in-progress: true|cancel-in-progress: \$/, 'no portability run is cancelled'); +}); + +test('real ComfyUI runs only outside pull requests; the other test.yml checks run on every event', () => { + const found = testJobs(); + assert.match(found.get('comfyui'), QUEUE_ONLY_IF); + for (const id of ['test', 'coverage', 'lint', 'js-test']) { + assert.doesNotMatch(found.get(id), /^ {4}if:/m, `${id} runs on every event`); + } +}); + +test('all tests needs every gating test.yml job and requires the full set outside pull requests', () => { + const found = testJobs(); + const gate = found.get('all-tests'); + assert.ok(gate, 'the aggregate gate exists'); + assert.match(gate, /^ {4}name: all tests$/m, 'the required context is "all tests"'); + assert.match(gate, /^ {4}if: always\(\)$/m); + const gating = [...found.keys()].filter(id => !['all-tests', 'perf-budget'].includes(id)); + assert.deepEqual([...needsOf(gate)].sort(), gating.sort(), 'the gate needs every job but the non-gating perf budget'); + assert.deepEqual([...gateVariables(gate).keys()].sort(), gating.sort(), 'every needed result reaches the gate'); + assertGateBehaviour(gate, ['comfyui']); +}); + +test('the CI workflows run on pull_request, merge_group and push without path filters', () => { + for (const name of GATING_WORKFLOWS) { + const text = readWorkflow(name); + const triggers = text.slice(text.indexOf('\non:'), text.indexOf('\n', text.indexOf('\non:') + 1) + 400); + for (const event of ['merge_group', 'pull_request', 'push']) { + assert.match(triggers, new RegExp(`^ {2}${event}:`, 'm'), `${name} runs on ${event}`); + } + assert.doesNotMatch(text, /^ {4}paths(-ignore)?:/m, `${name} has no path filter that would leave a required check pending`); + } + assert.doesNotMatch(jobs(readWorkflow('code-quality.yml')).get('security'), /^ {4}if:/m, 'bandit runs on every event'); +}); + +test('every required check reports on a pull request and the gates carry the full matrix', () => { + const contexts = JSON.parse(readRepoFile('.github/branch-protection.json')).required_status_checks.contexts; + assert.deepEqual([...contexts].sort(), [ + 'all tests', 'bandit (high severity)', 'black and flake8', 'coverage gate (85%)', 'frontend modules', + 'py3.12 on ubuntu-latest', 'real ComfyUI compatibility', 'viewerjs', + ].sort()); + assert.equal(contexts.filter(context => /^py/.test(context)).length, 1, + 'a dropped matrix leg never reports on a pull request, so it cannot be required'); + assert.ok(!contexts.some(context => /windows|macos/.test(context)), 'Windows and macOS never gate a merge'); +}); diff --git a/utils/exit_signal.py b/utils/exit_signal.py new file mode 100644 index 00000000..fa7d82b6 --- /dev/null +++ b/utils/exit_signal.py @@ -0,0 +1,110 @@ +"""The process exit signal: running work stops at its next safe point (#376). + +ComfyUI catches Ctrl+C in ``main.py`` and returns without closing its event +loop. The interpreter then runs ``threading._shutdown``, which first calls the +``threading._register_atexit`` callbacks, newest first. One of them, +``concurrent.futures``'s ``_python_exit``, joins every executor thread, and +only after that do the ``atexit`` callbacks run (the package teardown). A call +still running on the default executor (a scan batch, a model download, a +thumbnail batch) used to keep the process alive until it returned. + +``install()`` registers a callback in that same list after ``concurrent.futures`` +did, so it runs before the join. It sets the signal that ``requested()`` reads +(``Job.cancelled`` and ``map_parallel`` read it) and runs ``on_exit``. + +``threading._register_atexit`` is CPython-internal. It has the same contract +on 3.12, 3.13 and 3.14. Without it, or when it refuses the registration, exit +behaves as before: the call in flight runs to its end. +""" + +import logging +import threading + +logger = logging.getLogger("prompt_manager.exit") + +_requested = threading.Event() +_install_lock = threading.Lock() +_installed = False +_on_exit = [] + + +class ExitRequested(Exception): + """The interpreter is exiting: work not yet started is not started.""" + + +def requested(): + """True once the interpreter has started exiting.""" + return _requested.is_set() + + +def request(): + """Set the signal (the exit hook; tests).""" + _requested.set() + + +def checkpoint(): + """Raise ExitRequested once the interpreter is exiting.""" + if _requested.is_set(): + raise ExitRequested("ComfyUI is exiting") + + +def checked(func): + """``func`` behind a checkpoint: a call that starts after the signal + raises ExitRequested instead of running.""" + + def run(*args, **kwargs): + checkpoint() + return func(*args, **kwargs) + + return run + + +def _hook(): + """The threading atexit callback. It never raises: an exception here + would end ``threading._shutdown`` before it shuts the executors.""" + _requested.set() + for action in tuple(_on_exit): + try: + action() + except (Exception, KeyboardInterrupt): + logger.error("Stopping work at exit failed", exc_info=True) + + +def install(on_exit=None): + """Register the exit hook once per process; True when it is registered. + + ``on_exit`` (optional) runs inside the hook, after the signal is set. + False (logged at DEBUG) when this Python has no ``_register_atexit`` or + refuses it; exit then waits for running work as before. + """ + global _installed + with _install_lock: + if on_exit is not None and on_exit not in _on_exit: + _on_exit.append(on_exit) + if _installed: + return True + # Imported first so its pool join is registered before our hook: + # the callbacks run newest first, so ours runs before that join. + import concurrent.futures.thread # noqa: F401 + + register = getattr(threading, "_register_atexit", None) + if not callable(register): + logger.debug("threading._register_atexit is missing; exit waits for jobs") + return False + try: + register(_hook) + except RuntimeError as error: # already shutting down + logger.debug(f"Exit hook not registered: {error}") + return False + _installed = True + return True + + +def reset(): + """Clear the signal and forget the registration (tests only; a hook + already registered stays in ``threading`` and sets the signal at exit).""" + global _installed + with _install_lock: + _requested.clear() + _installed = False + _on_exit.clear() diff --git a/utils/parallel.py b/utils/parallel.py index 65dcca44..b790794b 100644 --- a/utils/parallel.py +++ b/utils/parallel.py @@ -21,6 +21,8 @@ import logging import threading from concurrent.futures import ThreadPoolExecutor +from .exit_signal import checked + SHUTDOWN_REFUSAL = "interpreter shutdown" _logger = logging.getLogger("prompt_manager.parallel") _noted = set() @@ -49,8 +51,11 @@ def map_parallel(func, items, workers): ``workers`` of 1 (or fewer) runs on the calling thread with no pool. An exception in ``func`` propagates to the caller, so callers that must survive bad input wrap ``func`` themselves. A pool refused at interpreter - exit runs the items it did not start on the calling thread (#375). + exit runs the items it did not start on the calling thread (#375). An item + that has not started when the interpreter starts exiting raises + ExitRequested instead (#376). """ + func = checked(func) items = list(items) workers = max(1, int(workers or 1)) if workers == 1 or len(items) <= 1: diff --git a/utils/prompt_inspector.py b/utils/prompt_inspector.py index 0307b5ca..1daaef6c 100644 --- a/utils/prompt_inspector.py +++ b/utils/prompt_inspector.py @@ -10,6 +10,7 @@ import threading import zlib from dataclasses import dataclass, field as dataclass_field from pathlib import Path +from collections.abc import Mapping from typing import Any from PIL import Image @@ -461,6 +462,14 @@ def read_inspection_metadata(path: Path) -> dict[str, str]: return read_inspection_file(path).metadata +# A side whose branch recorded run history named (#386): the save node that +# wrote the file picked the sampler; the text is still that branch's graph walk. +RECORDED_RUN_SOURCE = "recorded-run" +RECORDED_TEXT_REASON = "PromptManager recorded this text when the run executed" +# Outcomes the file alone leaves open; only these ask recorded history. +_UNDECIDED = frozenset({"ambiguous", "unresolved"}) + + def _side(status: str, text: str | None = None, source: str | None = None) -> dict: return {"status": status, "text": text, "source": source} @@ -529,15 +538,60 @@ def _parsed_workflow(value: Any) -> dict | None: return value if isinstance(value, dict) else None +def _recorded_execution(resolved: dict, texts: Mapping) -> dict | None: + """Evidence from the texts recorded at one sampler of the file's run (#386). + + Only for an outcome that names one sampler (a single stage stopped at a + run-time value, such as a language model's output feeding PromptManager) + and only when that sampler's positive text was recorded and is not empty. + """ + sampler = resolved.get("sampler_id") + roles = texts.get(sampler) if isinstance(sampler, str) else None + if not isinstance(roles, Mapping): + return None + positive = roles.get("positive") + if not isinstance(positive, str) or not positive.strip(): + return None + negative = roles.get("negative") + prompts = { + "positive": _side("resolved", positive, "execution"), + "negative": ( + _side("resolved", negative, "execution") + if isinstance(negative, str) + else _side("unresolved") + ), + } + resolution = { + "state": "resolved", + "sampler_id": sampler, + "reason": RECORDED_TEXT_REASON, + "reason_code": None, + "node": None, + } + return {"prompts": prompts, "resolution": resolution} + + def inspect_prompt_evidence( metadata: dict, saved: dict | None = None, *, profile: ResolverProfile = INSPECTION_PROFILE, file_name: str | None = None, + recorded_saver_id: str | None = None, + recorded_texts: Mapping[str, Mapping[str, str | None]] | None = None, ) -> dict: """Keep uncertainty visible; a saved association never replaces file evidence. + ``recorded_saver_id`` is the save node recorded run history says wrote the + file (#386). It is asked only when the file alone leaves the prompt open + (ambiguous or unresolved), and the anchored result is kept only when it + names the positive (resolved, or a guider with no negative); its + graph-inferred sides are then labelled ``recorded-run``. ``recorded_texts`` + (sampler id to the effective texts PromptManager recorded at execution in + that run) is used last: when the prompt is still open at one named sampler + whose positive text was recorded, the recorded texts are shown as + ``execution``. + Inspection routes resolve with ``INSPECTION_PROFILE``: bounded work, not graph size. Ingestion (scan, live monitor, admin import) calls the resolver directly with its default profile. ``resolution`` carries the closed @@ -599,6 +653,24 @@ def inspect_prompt_evidence( # Only to recognize overwritten PromptManager text (R1); never a role. canonical["workflow"] = workflow resolved = resolve_prompt_pair(canonical, profile=profile, file_name=file_name) + recorded = False + if resolved["state"] in _UNDECIDED and recorded_saver_id is not None: + anchored = resolve_prompt_pair( + canonical, + profile=profile, + file_name=file_name, + recorded_saver_id=recorded_saver_id, + ) + # 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"): @@ -608,9 +680,10 @@ def inspect_prompt_evidence( and isinstance(side, dict) and (role == "negative" or side["text"].strip()) ): - result[role] = _side( - "resolved", side["text"], side["provenance"]["text_source"] - ) + source = side["provenance"]["text_source"] + if recorded and source == "graph-inferred": + source = RECORDED_RUN_SOURCE + result[role] = _side("resolved", side["text"], source) elif state == "resolved" and isinstance(side, dict): # A resolved pair withholds only an empty positive. result[role] = {**_side("unresolved"), "message": _EMPTY_POSITIVE} diff --git a/utils/prompt_pair_resolver.py b/utils/prompt_pair_resolver.py index 15200481..ae871f19 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 @@ -1611,15 +1616,31 @@ class _GraphResolver: for node_id, node in self.graph.items() if self.writes(node, stem) ] - if len(matches) != 1: - return None - _, inputs = self.node(matches[0]) + except (_Unresolved, _Diverging): + return None + 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). + + The node is walked exactly like a file-name match. A node the graph + lacks, a failing walk or no candidate sampler is ``None``: unanchored. + """ + if not isinstance(self.graph.get(node_id), dict): + return None + self.budget.reset(self.per_candidate_visits) + return self.saver_walk(node_id) + + def saver_walk(self, saver_id: str) -> Optional[list]: + """The listed samplers in one saver's linked inputs' ancestry, or None.""" + try: + _, inputs = self.node(saver_id) values = [ value for name, value in inputs.items() if name != "filename_prefix" and _link(value) ] - found = self.stages_from(matches[0], values) + found = self.stages_from(saver_id, values) except (_Unresolved, _Diverging): return None candidates = [ @@ -2256,6 +2277,7 @@ def resolve_prompt_pair( max_depth: Optional[int] = None, max_text_length: Optional[int] = None, file_name: Optional[str] = None, + recorded_saver_id: Optional[str] = None, ) -> Dict[str, Any]: """Resolve parsed metadata; a present API graph is authoritative over text. @@ -2268,9 +2290,14 @@ def resolve_prompt_pair( ``profile`` selects the bounds; explicit ``max_*`` keywords override it. ``file_name`` (inspection only, unanchored) anchors on the one saver that wrote that file (``_GraphResolver.saver_stages``) when exactly one matches. + ``recorded_saver_id`` (inspection only, unanchored) is the save node that + recorded history says wrote the file (#386); it is tried before the file + name, and a node that yields no candidate sampler anchors nothing. """ profile = _profile_with(profile, max_visits, max_depth, max_text_length) - result = _resolve(metadata, output_node_id, adapters, profile, file_name) + result = _resolve( + metadata, output_node_id, adapters, profile, file_name, recorded_saver_id + ) return _legacy_result(result) if profile.legacy else result @@ -2324,6 +2351,7 @@ def _resolve( adapters: Optional[Mapping[Tuple[str, int], OutputAdapter]], profile: ResolverProfile, file_name: Optional[str] = None, + recorded_saver_id: Optional[str] = None, ) -> Dict[str, Any]: if not isinstance(metadata, dict) or not metadata: return _result("absent", reason="No parsed prompt metadata") @@ -2379,11 +2407,12 @@ def _resolve( "unresolved", reason=str(error), reason_code=error.code, node=error.node ) else: - anchored = ( - resolver.saver_stages(file_name) - if semantics is not None and file_name is not None - else None - ) + anchored, by_record = None, False + if semantics is not None and recorded_saver_id 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 for node_id, node in normalized.items() @@ -2393,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/admin.html b/web/admin.html index 2f064a3c..e55fbda6 100644 --- a/web/admin.html +++ b/web/admin.html @@ -164,27 +164,31 @@ class="px-2.5 py-1 bg-pm-surface border border-pm hover:bg-pm-hover text-pm text-xs font-medium rounded-pm-sm transition-colors"> Export - + Diagnostics - - - + Loading... @@ -369,61 +374,13 @@ - - -