diff --git a/TESTING.md b/TESTING.md index df15a12..193d591 100644 --- a/TESTING.md +++ b/TESTING.md @@ -2016,8 +2016,36 @@ browser fields, then the host fields (`POST /prompt_manager/settings` with highlights their controls; `settings-changed` is a conflict and keeps the draft. System reads the host values on mount and again after a conflict; until a read returned a revision, a host save fails without a request, so it never sends a blind -`expected_revision: null` (which S1 merges into the file on disk). No delivered section -edits a setting yet, so Save stays disabled until Appearance (S3). +`expected_revision: null` (which S1 merges into the file on disk). Save stays disabled +until a section has an unsaved change. + +Appearance (`system-appearance.mjs`, S3) is the concept's "Browsing & display" surface +on real values. Its browser rows are this browser's display defaults in the legacy +gallery's own `gallerySettings` object (`web/js/workspace/preferences.mjs`, Q3): Default +layout and Thumbnail size (`workspaceLayout` grid/list/masonry, `workspaceDensity` +comfortable/large/compact, new keys), Load metadata on selection (`autoLoadMetadata`), +Cache metadata in this browser (`cacheMetadata`), Image loading quality (`imageQuality`: +Optimized→`medium`, Original→`high`, Fast→`low`; the thumbnail dialog's preselected quality +once S5 lands), the three video rows (`videoAutoplay`, `videoMute`, `videoLoop`) and Show +file paths (`showFilePaths`). A write re-reads the stored object and writes back only the +changed keys, every other key (legacy paging, thumbnail state, unknown keys) as stored; +the legacy gallery's Save also keeps the keys it does not edit, so the two pages stay in +sync. The workspace never reads the legacy paging keys (`defaultLimit`, `gridColumns`, +`infiniteScroll`; README continuous-browsing ruling). A browser that refuses storage +(denied, or a full quota that leaves the stored bytes as they were) keeps the change for +this tab only and the status says "Saved for this tab only; browser storage is +unavailable.", never "Saved in this browser.". Another tab's change (a `storage` event) +repaints the clean controls; a draft it overtook stays, highlighted, with the conflict +status. Open from ComfyUI (New tab→`newtab`, Popup window→`popup`) and Node search +auto-hide (seconds) (a whole number 0–300; anything else is highlighted and never sent) +are the server's `webui_display_mode` and `result_timeout`, saved through +`POST /prompt_manager/settings` with the revision. The consumers read the defaults on +their next visit, never the current view: a new Images visit starts from the layout and +size (the last visit keeps its own, a saved view opens at Grid and Comfortable as +before); with metadata off a selection reads nothing until "Load metadata"; with the +cache off selecting the shown file again re-reads it (on, the one current record is +reused); paths off leaves out only the Path row of File & provenance; the viewer reads +the video defaults. Overview (`system-overview.mjs`) reads `GET /prompt_manager/system/overview` once per visit through `latestOnly()` on the section scope; a late answer after the section left @@ -2046,7 +2074,7 @@ the connection's `total_changes` is 0 at close and the data reads back unchanged `-shm` bytes are not compared: every WAL reader rewrites its read marks there, so no reader can keep it byte-identical (`test_health_check_never_writes`). -Recorded deviations (plan D60–D64 renumbered D92–D96; D121, D122 and D123 assigned): +Recorded deviations (plan D60–D65 renumbered D92–D97; D121, D122, D123 and D124 assigned): - **D92** (shell): no "Sample configuration" chip. The Save status is truthful: "No unsaved changes.", "Unsaved changes.", "Saved in this browser." / "Saved to the server @@ -2106,6 +2134,36 @@ Recorded deviations (plan D60–D64 renumbered D92–D96; D121, D122 and D123 as the rest of Maintenance, above its D94 hand-off, as the native place that runs the health check. A press shows "Checking…" (`aria-disabled`) and opens the report; a failed check toasts "The health check could not finish. Nothing was changed." +- **D97** (Appearance): added rows for the legacy gallery settings the concept leaves + out, after its own rows: "Show image details on cards" (`showImageInfo`; off hides a + card's title and file line, Info and Tag stay at the end of the row), "Collapse viewer + details by default" (`sidebarCollapsedByDefault`; the viewer opens with + `#viewer-details` hidden and a "Show details"/"Hide details" toggle with + `aria-expanded`, the stage taking the whole width), and the operation "Reset display + defaults" ("Restore this browser’s display defaults, then save. Other settings are + kept.", button "Reset"), which drafts the gallery's defaults for the browser rows only. + The node search timeout shows the server's real value, not the sample 30. CSS under D97. +- **D124** (inspector): with "Load metadata on selection" off, the Images panel and + the viewer's details show "Metadata loads when you ask for it." and a "Load metadata" + button instead of reading the file. The concept has no such state, so no concept + comparison renders it: `metadata off shows an explicit load and loads once on request` + (workspace-system.spec) pins it instead. + +Appearance scope and states. The Default layout and Thumbnail size defaults start the +Images page only (a new visit, not the current view or a saved view); the embedded +galleries (a prompt history's Shared gallery and its All images / Repeat run dialogs) +have no toolbar, so they keep Grid/Comfortable, while the metadata, cache and card +defaults apply there too. A host row whose `/settings` read failed stays disabled with +"Server settings could not be read."; a rejected timeout says "Some changes were not +saved. Review the highlighted settings." A display default another page saved under one +of this tab's drafts flags that draft with the conflict status, whether it arrived while +System was mounted (a `storage` event) or while it was not (checked again on mount, and +before a write); the draft is never written over the other page's value without that +status first. A change kept for this tab only is stored by the next write storage accepts. + +Open question (saved-views follow-up, M8): a saved view stores no layout or size and +always opens at Grid/Comfortable, even when the Default layout is Masonry. Should saved +views store a layout, or follow the default? `tests/browser/system-concept.spec.mjs` compares the shell and Overview with the concept (dark and light, 1600×1100 and 390×844), mapping only the deviations above; unmapped, @@ -2118,20 +2176,34 @@ source when there are several, replaces only the chips with the fixture's expect results, and asserts the full unmapped path list; it maps D96 and compares the Maintenance notice unchanged. `system comparison detects mutations` removes a row, renames a metric label, swaps the `--line` token, recolours a chip, renames a check, -removes the totals table and renames a caption. Paired captures (`{concept,production}-system-{dark,light}-{1600,390}.png`, -`{concept,production}-{statistics,health}-{dark,light}.png`) are test artifacts; there +removes the totals table and renames a caption. `appearance matches the concept` maps +only D97 and the real timeout (unmapped, exactly those paths differ), and `appearance +comparison detects mutations` removes a row, renames a label and an option and swaps +the `--line` token. Paired captures (`{concept,production}-system-{dark,light}-{1600,390}.png`, +`{concept,production}-{statistics,health}-{dark,light}.png`, +`{concept,production}-appearance-{dark,light}-{1600,390}.png`) are test artifacts; there are no PNG baselines. `tests/browser/workspace-system.spec.mjs` covers the real counts, the health check and metric 4, a failed check, the thumbnail row against the status endpoint (painted after the rest when its read is held, unavailable when it fails), the review row (Q6), the section links, the Statistics dialog against the -route, a load failure with Retry and late answers after leaving. Its hooks are +route, a load failure with Retry and late answers after leaving, and Appearance: new +visits versus the current view and a saved view, the embedded gallery keeping Grid, +metadata off and on request, the cache (counted up to a request barrier), hidden paths, +the video defaults, the host fields with their revision, a stale one and a failed read, +reload and a second tab, a second tab's save while System was closed, storage refused, +the D97 rows and reset. Its hooks are `tests/browser/workspace_system_fixture.py` (`/__test__/system/*`: reset, injected failures, an absent source, a deleted linked file, unthumbnailed files) on the -concept-shaped fixture, with delays through the shared request holds. +concept-shaped fixture, with delays through the shared request holds; reset also restores +the server's host settings and its `config.json`, which must be the one inside the server's +temp root (`tests/test_workspace_system_fixture.py`). ```sh -python3 -m unittest tests.test_database_health tests.test_system_api tests.test_library -v +python3 -m unittest tests.test_database_health tests.test_system_api tests.test_library \ + tests.test_workspace_system_fixture -v node --test tests/js/workspace-system-state.test.js tests/js/workspace-system-controls.test.js \ - tests/js/workspace-system-overview.test.js tests/js/workspace-state.test.js + tests/js/workspace-system-overview.test.js tests/js/workspace-state.test.js \ + tests/js/workspace-system-appearance.test.js tests/js/workspace-preferences.test.js \ + tests/js/workspace-inspection-state.test.js tests/js/workspace-images-state.test.js npm run --prefix tools/vendor test:workflows -- system-concept.spec.mjs workspace-system.spec.mjs \ --project chromium --project touch --project firefox --project webkit ``` @@ -2323,10 +2395,24 @@ validation, annotation group included. The browser fixtures clear never the metadata row. Needs review items (#249, `utils/review_capture.py`, `database/review.py`, -`tests/test_review.py`): the output scan (Activity's Scan for metadata and the -legacy `/scan`) and the live monitor hand every file the ingestion resolver -finds `ambiguous` to `record_ambiguous_file`, whether or not the browser parser -then links it (`conflict` and `unresolved` files are not queued). It captures +`tests/test_review.py`, `tests/test_review_outcome.py`): a file is queued only +when its final outcome is ambiguous and unlinked. The output scan (Activity's +Scan for metadata and the legacy `/scan`) hands `record_ambiguous_file` a file +the ingestion resolver finds `ambiguous` only when the browser-parser fallback +then links nothing (status `ambiguous`) and no saved row links the file +already; an ambiguous file a saved row links is reported `unchanged` ("Already +linked to its prompt"), not Needs review. A file the fallback links, an exact +pair the ingester stores, and `conflict` and `unresolved` files are not +queued. The queue heals itself: after each batch, the scan removes the pending +items of every listed file (read or skipped as known) that a saved row now +links, and the monitor does the same for each ambiguous output it links +(`settle_linked_review_items`; the review index revision moves). Only an +undecided item goes (`pending`, revision 0, no decision row): a decision is +never deleted or changed, so no schema state was added. The monitor never +queues an output it linked; an unlinked file passed to `record_review_file` +follows the scan's rule (a file the fallback would link is `skipped`). The +maintainer's live check of #284/#288 found the queue at 10,232 against a review +group of 4,677: every fallback-linked ambiguous file had been queued. It captures the file (scope `needs-review`), reads its metadata again from the capture, resolves it with the default profile and stores only an ambiguous result, bound to the capture's byte record (created or refreshed like an observe) and @@ -2340,14 +2426,20 @@ resolver_version, evidence_hash)` and never touches an existing item or its decisions; changed bytes add a new pending item and keep the old one. The capture is released before the write; no model is loaded and no pixel is decoded. Scan statuses, counters, outcome rows and the legacy SSE events are -unchanged; the job's `scope` gains `review_items` (new items) and -`review_errors`, and the log ends with `? N images added to Needs review` (and -`! N images could not be added to Needs review` on failures, e.g. capture -capacity). Recording captures in the background: it never takes the store's +unchanged apart from the already-linked case above; the job's `scope` gains +`review_items` (this scan's files that are in the pending queue after it, new +or already there: the review group's ambiguous rows, less files a capture could +not read or a decision already settled), `review_added` (new items), +`review_settled` (items removed because their file is linked now) and +`review_errors`. The log ends with `? N images added to Needs review`, +`? N images already in Needs review`, `= N images left Needs review · now +linked` and `! N images could not be added to Needs review` (failures, e.g. +capture capacity), each only when N > 0. Recording captures in the background: it never takes the store's last worker or capture slot, which stay free for an interactive Tag dialog or Apply, and waits up to 2 s for room before counting the file as not added -(`test_background_recording_leaves_a_worker_for_the_tag_dialog`). The monitor records after its fallback link, best effort: a failure -is logged and the link stays. Recording follows the registry's `records` +(`test_background_recording_leaves_a_worker_for_the_tag_dialog`). The monitor's +pass runs after its own link, best effort: a failure is logged and the link +stays. Recording follows the registry's `records` admission (#289): the scan's recording runs under its own `records` job, and the monitor's (one file, not a job) is refused while a `records` or `guarded` job or hold runs. It then records nothing, logs that the file waits for the diff --git a/__init__.py b/__init__.py index 78da47a..6123a47 100644 --- a/__init__.py +++ b/__init__.py @@ -125,9 +125,10 @@ try: _global_prompt_tracker = get_prompt_tracker(_global_db) _global_image_monitor = get_image_monitor(_global_db, _global_prompt_tracker) - # Ambiguous new outputs go to Needs review through the API's capture store - # (the one the scan and the Tag dialog use). Recording stops before that - # store is closed at shutdown. + # The Needs review pass for ambiguous new outputs runs through the API's + # capture store (the one the scan and the Tag dialog use): a linked output + # is never queued and its pending items leave the queue. It stops before + # that store is closed at shutdown. _review_api = globals().get("api") if _review_api is not None: _global_image_monitor.set_review_recorder(_review_api.record_review_file) diff --git a/database/review.py b/database/review.py index 5f15bb7..af06c72 100644 --- a/database/review.py +++ b/database/review.py @@ -33,6 +33,7 @@ from .annotations import _new_id, _validate_locator, _validate_target from .models import normalize_image_path, utc_now_iso from .review_changes import notify_review_change from .review_schema import ( + DECISIONS_TABLE, ITEMS_TABLE, MAX_CANDIDATES, MAX_SPELLED_PATH_BYTES, @@ -81,6 +82,26 @@ _QUEUED = ( "WHERE f.locator=? AND b.size=? AND b.mtime_ns=? AND i.resolver_version=? " "LIMIT 1" ) +_PENDING_AT = ( + f"SELECT 1 FROM {FILES_TABLE} f " # nosec B608 + f"JOIN {BYTES_TABLE} b ON b.file_id=f.id " + f"JOIN {ITEMS_TABLE} i ON i.byte_id=b.id " + "WHERE f.locator=? AND b.size=? AND b.mtime_ns=? AND i.resolver_version=? " + "AND i.state='pending' LIMIT 1" +) +# Pending items of one file: the candidates a link settles. +_PENDING_OF_LOCATOR = ( + f"SELECT i.id FROM {FILES_TABLE} f " # nosec B608 + f"JOIN {BYTES_TABLE} b ON b.file_id=f.id " + f"JOIN {ITEMS_TABLE} i ON i.byte_id=b.id " + "WHERE f.locator=? AND i.state='pending'" +) +# Only an undecided item goes: a decision row (RESTRICT) is never deleted. +_SETTLE_ITEM = ( + f"DELETE FROM {ITEMS_TABLE} WHERE id=? AND state='pending' " # nosec B608 + f"AND revision=0 AND NOT EXISTS (SELECT 1 FROM {DECISIONS_TABLE} d " + f"WHERE d.item_id={ITEMS_TABLE}.id)" +) _EPOCH = "SELECT storage_epoch FROM execution_history_metadata WHERE singleton_id=1" _PENDING_COUNT = ( f"SELECT COUNT(*) FROM {ITEMS_TABLE} WHERE state='pending'" # nosec B608 @@ -360,6 +381,60 @@ class ReviewOperations: ) return row is not None + def review_pending_at( + self, locator: str, size: int, mtime_ns: int, resolver_version: int + ) -> bool: + """True when a *pending* item cites bytes verified at this exact stat. + + ``review_fingerprint_queued`` also matches decided items; a file whose + only item is decided is not in the queue. + """ + row = ( + self.model.get_connection() + .execute(_PENDING_AT, (locator, size, mtime_ns, resolver_version)) + .fetchone() + ) + return row is not None + + def review_file_linked(self, spellings) -> bool: + """True when a saved row links the file under any of ``spellings``.""" + with self._annotation_read() as conn: + return bool(dependent_images(conn, spellings)) + + def settle_linked_review_items(self, files) -> int: + """Remove the pending, undecided items of files that are now linked. + + ``files`` holds ``(locator, spellings)`` per file. An item leaves the + queue only when it is still ``pending`` at revision 0 with no decision + row and a saved row links its file under one of ``spellings``; a + decided item is never touched. Files without a pending item cost one + indexed read and open no write transaction. Returns how many items + were removed (the review index revision moves when any was). + """ + files = [(locator, tuple(spellings)) for locator, spellings in files] + for locator, _spellings in files: + _validate_locator(locator) + conn = self.model.get_connection() + candidates = [ + (locator, spellings) + for locator, spellings in files + if conn.execute(_PENDING_OF_LOCATOR, (locator,)).fetchone() is not None + ] + if not candidates: + return 0 + removed = 0 + with self._pair_transaction() as conn: + for locator, spellings in candidates: + if not dependent_images(conn, spellings): + continue + for (item_id,) in conn.execute( + _PENDING_OF_LOCATOR, (locator,) + ).fetchall(): + removed += conn.execute(_SETTLE_ITEM, (item_id,)).rowcount + if removed: + conn.execute(_BUMP_REVIEW_INDEX) + return removed + def review_queue_page(self, *, limit: int, after: tuple | None = None) -> dict: """One page of pending items, oldest first, from one snapshot. diff --git a/py/api/admin.py b/py/api/admin.py index b40a60b..740643a 100644 --- a/py/api/admin.py +++ b/py/api/admin.py @@ -211,6 +211,28 @@ def _time_left(done, total, batches, elapsed): return f" · about {minutes} min left" +def new_review_counts(): + """The scan's Needs review tally: ``added`` new items, ``queued`` files + in the queue after this scan (new or already there), ``settled`` items + removed because their file is linked now, and ``errors``.""" + return {"added": 0, "queued": 0, "settled": 0, "errors": 0} + + +def _review_scope(review): + """The scan job's ``scope`` keys for its Needs review tally. + + ``review_items`` is the number of this scan's files that are in the + pending queue after it: the review group's ambiguous files, less those a + capture could not read or a decision already settled. + """ + return { + "review_items": review["queued"], + "review_added": review["added"], + "review_settled": review["settled"], + "review_errors": review["errors"], + } + + def _scan_status_line(done, total, batches, elapsed): """ "Processing file N of M", with an estimate once enough batches ran.""" line = f"Processing file {done:,} of {total:,}" @@ -1445,8 +1467,8 @@ class AdminRoutesMixin: known = set() if media_files and job.scope.get("files") != "all": known = await self._known_scan_files(job, media_files) - review = {"recorded": 0, "errors": 0} - job.scope.update(review_items=0, review_errors=0) + review = new_review_counts() + job.scope.update(_review_scope(review)) async for event in self._scan_batches( media_files, counts, job, roots, known, review ): @@ -1524,11 +1546,12 @@ class AdminRoutesMixin: ) for key in counts: counts[key] += batch_counts[key] - if review is not None: - job.scope.update( - review_items=review["recorded"], - review_errors=review["errors"], - ) + if review is not None: + # After the batch's links: a file linked now leaves the queue. + await self._run_in_executor( + self._settle_scanned_reviews, batch, roots, review + ) + job.scope.update(_review_scope(review)) for group, row, log_line in outcomes: job.record(group, row, log_line) @@ -1585,10 +1608,11 @@ class AdminRoutesMixin: Returns the legacy counts. When ``outcomes`` is a list, every processed file appends its ``(group, row, log_line)`` for the scan job, with the - path relative to its root in ``roots`` (see ``_scan_roots``). A file the - resolver finds ambiguous is also recorded for Needs review (best - effort, counted in ``review`` when given); statuses and counts do not - depend on it. + path relative to its root in ``roots`` (see ``_scan_roots``). A file + whose final outcome is ambiguous and unlinked (the resolver found it + ambiguous and the fallback did not link it) is recorded for Needs + review (best effort, counted in ``review`` when given); one a saved + row already links is reported ``unchanged`` and never queued. """ counts = dict.fromkeys(SCAN_COUNT_KEYS, 0) @@ -1610,8 +1634,11 @@ class AdminRoutesMixin: counts["failed"] += 1 note(media_file, "error") continue - if ambiguous: - self._record_scanned_review(media_file, roots, review) + if ambiguous and outcome == "ambiguous": + if self._review_file_linked(media_file, roots): + outcome = "unchanged" # already linked: not a review case + else: + self._record_scanned_review(media_file, roots, review) if outcome is None: note(media_file, None) continue @@ -1655,7 +1682,8 @@ class AdminRoutesMixin: ``status``, the prompt's display ``title`` and, for a resolved pair, the pair counts. When the resolver finds the file ambiguous (whether or not the browser parser then links it) and ``ambiguous`` is a list, - ``media_file`` is appended to it. + ``media_file`` is appended to it; only a file that then returns + ``"ambiguous"`` (nothing linked it) is a Needs review case. """ if not metadata: return None diff --git a/py/api/review_recording.py b/py/api/review_recording.py index 22d0c16..31de6d9 100644 --- a/py/api/review_recording.py +++ b/py/api/review_recording.py @@ -1,16 +1,29 @@ """Needs review recording for the output scan and the live monitor (#249). -The scan hands every file the ingestion resolver finds ambiguous to -``_record_scanned_review`` (best effort: a failure is logged and counted); -the monitor calls ``record_review_file`` from its worker thread after its -fallback link. Both go through ``utils.review_capture.record_ambiguous_file`` -on the API's capture store. Nothing here links a file. +The rule (maintainer test of #284/#288): a file is queued only when its +final outcome is ambiguous and unlinked. The strict resolver must find it +``ambiguous``, the scan's legacy fallback (the browser parser, +``_graph_prompt_text``) must not link it, and no saved row may link it +already. A file the fallback links, an exact pair the ingester stored and an +already linked file are never queued. + +The queue heals itself: a pending item whose file is now linked (by the +fallback, a later resolution or the monitor) is removed by the next scan +that lists the file or by the monitor's next pass over it. Only an +undecided item goes (``pending``, revision 0, no decision row); a decision +is never deleted or changed (``settle_linked_review_items``). + +The scan records through ``_record_scanned_review`` (best effort: a failure +is logged and counted); the monitor calls ``record_review_file`` from its +worker thread after its own link. Both go through +``utils.review_capture.record_ambiguous_file`` on the API's capture store. +Nothing here links a file. Recording changes records, so it follows the registry's ``records`` admission (#289). The scan records under its own ``records`` job. The -monitor's recording lasts one file and is not a job: it is refused while a +monitor's pass lasts one file and is not a job: it is refused while a ``records`` or ``guarded`` job runs (``deferred``), and the next scan reads -the ambiguous file again. +the file again. """ import os @@ -20,6 +33,9 @@ from .admin import SCAN_LOG_PREFIXES, _plural, _scan_location, _scan_roots # The kind the monitor's recording checks admission under (never registered). REVIEW_RECORDING_KIND = "review_recording" +# record_ambiguous_file outcomes that leave the file in the pending queue. +QUEUED_OUTCOMES = ("recorded", "limited", "duplicate") +NEW_OUTCOMES = ("recorded", "limited") try: from ...utils.review_capture import record_ambiguous_file @@ -31,7 +47,7 @@ class ReviewRecordingMixin: """Record the scan's and the monitor's ambiguous files for Needs review.""" def _record_scanned_review(self, media_file, roots, review=None): - """Record one ambiguous scanned file; a failure is logged and counted.""" + """Record one ambiguous, unlinked scanned file (logged and counted).""" try: outcome = self._record_review_sync(media_file, roots) except Exception: @@ -42,48 +58,130 @@ class ReviewRecordingMixin: if review is not None: review["errors"] += 1 return - if review is not None and outcome in ("recorded", "limited"): - review["recorded"] += 1 + if review is None: + return + if outcome in NEW_OUTCOMES: + review["added"] += 1 + if outcome in QUEUED_OUTCOMES: + review["queued"] += 1 def _record_review_sync(self, media_file, roots): """``record_ambiguous_file`` for a file under one of ``roots`` (blocking).""" - relative, _label, index = _scan_location(media_file, roots) - root = next((r for r, _l, i in roots if i == index), None) - if index is None or root is None: + located = self._review_location(media_file, roots) + if located is None: return "skipped" + root, index, relative, _spelled = located return record_ambiguous_file( self.db, self._capture_store(), Path(root), index, relative ) - def record_review_file(self, image_path): - """Needs review recording for the live monitor (its worker thread). + @staticmethod + def _review_location(media_file, roots, real_roots=None): + """``(root, root_index, relative, spelled)`` of a listed file, or None. - Maps ``image_path`` to the gallery root that lists it (the listing's - ``root_index``); a file under no current root is ``skipped``. While a - job runs that a ``records`` job may not run beside, nothing is recorded - and the answer is ``deferred``. Errors propagate: the monitor logs them - and keeps its link. + ``spelled`` is the capture's spelling of the file (the real root joined + with the relative path); ``os.path.normcase`` of it is the locator + review items are stored under. ``real_roots`` caches each root's + ``realpath`` across a batch. + """ + relative, _label, index = _scan_location(media_file, roots) + root = next((r for r, _l, i in roots if i == index), None) + if index is None or root is None: + return None + if real_roots is None: + real_roots = {} + real = real_roots.get(index) + if real is None: + real = real_roots[index] = os.path.realpath(os.fspath(root)) + spelled = os.path.join(real, *relative.split("/")) + return root, index, relative, spelled + + def _review_file_linked(self, media_file, roots): + """True when a saved row links the scanned file (any spelling).""" + located = self._review_location(media_file, roots) + spellings = (str(media_file),) + ((located[3],) if located else ()) + return self.db.review_file_linked(spellings) + + def _settle_scanned_reviews(self, media_files, roots, review=None): + """Remove the pending items of ``media_files`` that are now linked. + + Blocking; best effort like recording (a failure is logged and + counted in ``review["errors"]``). Returns how many items left. + """ + real_roots = {} + files = [] + for media_file in media_files: + located = self._review_location(media_file, roots, real_roots) + if located is not None: + spelled = located[3] + files.append((os.path.normcase(spelled), (str(media_file), spelled))) + try: + settled = self.db.settle_linked_review_items(files) if files else 0 + except Exception: + self.logger.warning( + "Needs review items of linked files could not be removed", + exc_info=True, + ) + if review is not None: + review["errors"] += 1 + return 0 + if review is not None: + review["settled"] += settled + return settled + + def _fallback_links(self, metadata): + """True when the scan's legacy fallback would link these records.""" + return bool(metadata.get("prompt")) and bool(self._graph_prompt_text(metadata)) + + def record_review_file(self, image_path): + """Apply the Needs review rule to one monitor output (its worker thread). + + Maps ``image_path`` to the gallery root that lists it; a file under no + current root is ``skipped``. A linked file is never queued: its + pending items are removed and the answer is ``linked``. An unlinked + file the scan's fallback would link is ``skipped`` (the next scan + links it); any other file is recorded when it is ambiguous. While a + job runs that a ``records`` job may not run beside, nothing is written + and the answer is ``deferred``. Errors propagate: the monitor logs + them and keeps its link. """ busy = self._jobs.blocker(REVIEW_RECORDING_KIND, admission="records") if busy is not None: self.logger.info( - f"{os.path.basename(str(image_path))} was not added to Needs " - f"review while {busy.title} runs; the next scan reads it again" + f"Needs review was not updated for " + f"{os.path.basename(str(image_path))} while {busy.title} runs; " + "the next scan reads it again" ) return "deferred" full = os.path.abspath(str(image_path)) located = Path(os.path.realpath(os.path.dirname(full))) / os.path.basename(full) - return self._record_review_sync( - located, _scan_roots(self._get_all_output_dirs()) - ) + roots = _scan_roots(self._get_all_output_dirs()) + if self._review_location(located, roots) is None: + return "skipped" + if self._review_file_linked(located, roots): + self._settle_scanned_reviews([located], roots) + return "linked" + if self._fallback_links(self._extract_comfyui_metadata(str(located))): + return "skipped" + return self._record_review_sync(located, roots) @staticmethod def _log_review_items(job, review): """The scan log's Needs review summary lines (none when nothing moved).""" - if review["recorded"]: + prefix = SCAN_LOG_PREFIXES["review"] + if review["added"]: job.log_line( - f"{SCAN_LOG_PREFIXES['review']} " - f"{_plural(review['recorded'], 'image')} added to Needs review" + f"{prefix} {_plural(review['added'], 'image')} added to Needs review" + ) + already = review["queued"] - review["added"] + if already: + job.log_line( + f"{prefix} {_plural(already, 'image')} already in Needs review" + ) + if review["settled"]: + job.log_line( + f"{SCAN_LOG_PREFIXES['known']} " + f"{_plural(review['settled'], 'image')} left Needs review · now linked" ) if review["errors"]: job.log_line( diff --git a/py/api/system.py b/py/api/system.py index 605334e..092042e 100644 --- a/py/api/system.py +++ b/py/api/system.py @@ -41,12 +41,11 @@ _HEALTH_STATUS_LINES = { _StoredReport = namedtuple("_StoredReport", "generation report") -async def _settled(*reads): - """Every read's result once all of them ended; the first error raises. +async def _gather_settled(*reads): + """Await every read, then raise the first failure. - ``asyncio.gather`` answers at the first error while the other executor - reads still run, so a route could answer (and its caller replace or - delete the database) with a sibling read still inside SQLite. + Unlike a plain gather, a failure never answers while the other executor + reads still run against the database. """ results = await asyncio.gather(*reads, return_exceptions=True) for result in results: @@ -169,7 +168,7 @@ class SystemRoutesMixin: from ..config import GalleryConfig try: - (output_dirs, auto_detected), counts = await _settled( + (output_dirs, auto_detected), counts = await _gather_settled( self._run_in_executor(self._output_dirs_with_origin), self._run_in_executor(self.db.library_counts), ) @@ -203,7 +202,7 @@ class SystemRoutesMixin: async def get_system_statistics(self, request): try: - by_category, totals, counts = await _settled( + by_category, totals, counts = await _gather_settled( self._run_in_executor(self.db.usage_by_category), self._run_in_executor(self.db.get_statistics), self._run_in_executor(self.db.library_counts), diff --git a/tests/browser/system-concept.spec.mjs b/tests/browser/system-concept.spec.mjs index 16a7372..9ec92ba 100644 --- a/tests/browser/system-concept.spec.mjs +++ b/tests/browser/system-concept.spec.mjs @@ -407,3 +407,128 @@ for (const theme of THEMES) test(`statistics and health dialogs match the concep await page.evaluate(() => { document.querySelector('#modal-body caption').textContent = 'Runs per category'; }); expect(diffInventory(expectedStatistics, await page.evaluate(measureDialog)).map(item => item.path)).toEqual(['tables.0.caption']); }); + +/** Runs in the page: Appearance's surface, every row's text and control, and the concept's row geometry and tokens. */ +function measureAppearance(mainSelector) { + const layout = document.querySelector(`${mainSelector} .settings-layout`); + const css = (node, ...keys) => node ? Object.fromEntries(keys.map(key => [key, getComputedStyle(node)[key]])) : null; + const name = node => [node.localName, ...[...node.classList].sort()].join('.'); + const text = node => node?.textContent.trim() ?? null; + const surface = layout.querySelector(':scope > section.surface'); + const control = row => { + const select = row.querySelector(':scope > select'); + if (select) return { kind: 'select', id: select.id, options: [...select.options].map(option => option.textContent), + selected: select.selectedOptions[0]?.textContent ?? null }; + const input = row.querySelector(':scope > input'); + if (input) return { kind: `input.${input.type}`, id: input.id, value: input.value }; + const toggle = row.querySelector(':scope > * input[type="checkbox"]'); + if (toggle) return { kind: 'switch', id: toggle.id, wrapper: name(toggle.parentElement), checked: toggle.checked, + ...css(toggle.parentElement, 'width', 'height') }; + const chip = row.querySelector(':scope > .chip'); + return chip ? { kind: name(chip), text: text(chip), ...css(chip, 'color', 'backgroundColor', 'fontSize') } : null; + }; + const rows = [...surface.querySelectorAll(':scope > .setting-row')]; + const operations = [...surface.querySelectorAll(':scope > .operation')]; + const firstRow = rows[0]; + return { + layout: [...layout.children].map(name), + surface: { tree: [...surface.children].map(name), heading: text(surface.querySelector(':scope > h2')), + intro: text(surface.querySelector(':scope > p')), + ...css(surface, 'paddingTop', 'paddingLeft', 'backgroundColor', 'borderTopWidth', 'borderTopColor', 'borderTopLeftRadius') }, + rows: rows.map(row => ({ tree: [...row.children].map(name), text: [...row.firstElementChild.children].map(node => [node.localName, text(node), + node.localName === 'label' ? node.htmlFor : null]), control: control(row) })), + operations: operations.map(row => ({ tree: [...row.children].map(name), title: text(row.querySelector('h3')), + description: text(row.querySelector('p')), action: text(row.querySelector('.row-actions > button')) })), + row: css(firstRow, 'display', 'alignItems', 'justifyContent', 'columnGap', 'paddingTop', 'paddingBottom', 'borderTopWidth', + 'borderTopColor', 'marginTop'), + label: css(firstRow.querySelector('label'), 'fontSize', 'fontWeight', 'color'), + description: css(firstRow.querySelector('p'), 'fontSize', 'color', 'marginTop', 'maxWidth'), + select: css(firstRow.querySelector('select'), 'fontSize', 'minWidth', 'maxWidth', 'backgroundColor', 'color', 'borderTopColor'), + heading3: css(surface.querySelector('.setting-row h3'), 'fontSize', 'fontWeight', 'color'), + number: css(surface.querySelector('.setting-row input[type="number"]'), 'fontSize', 'minWidth', 'maxWidth', 'backgroundColor'), + pageScroll: document.documentElement.scrollWidth > document.documentElement.clientWidth, + }; +} + +// D97: the rows Appearance adds for the legacy gallery settings, after the concept's own, as production renders them. +const ADDED_APPEARANCE_ROWS = [ + ['card-details', 'Show image details on cards', 'Show each card’s title and file details under the preview.', true], + ['viewer-details', 'Collapse viewer details by default', 'Open the image viewer with its details panel hidden.', false], +]; + +/** The concept's Appearance with the recorded deviations applied, given the server's real settings. */ +function mapAppearanceConcept(reference, settings) { + const toggleRow = reference.rows.find(row => row.control?.kind === 'switch'); + const added = ADDED_APPEARANCE_ROWS.map(([id, label, description, checked]) => ({ + tree: toggleRow.tree, text: [['label', label, id], ['p', description, null]], + control: { ...toggleRow.control, id, checked } })); + return { + ...reference, + // D97: two added toggles and the "Reset display defaults" operation close the surface. + surface: { ...reference.surface, tree: [...reference.surface.tree, 'div.setting-row', 'div.setting-row', 'div.operation'] }, + rows: [...reference.rows.map(row => row.control?.id === 'timeout' + // The node search timeout is the server's real `result_timeout`, not the concept's sample 30. + ? { ...row, control: { ...row.control, value: String(settings.result_timeout) } } : row), ...added], + operations: [{ tree: ['div', 'div.row-actions'], title: 'Reset display defaults', + description: 'Restore this browser’s display defaults, then save. Other settings are kept.', action: 'Reset' }], + }; +} + +async function conceptAppearance(page, theme) { + await openConcept(page, 'system', theme); + await page.locator('#main .section-nav').getByRole('button', { name: 'Appearance', exact: true }).click(); + await expect(page.locator('#main .settings-layout > section.surface > h2')).toHaveText('Browsing & display'); + await page.mouse.move(0, 0); + return page.evaluate(measureAppearance, '#main'); +} + +async function productionAppearance(page, theme) { + await openProduction(page, 'system/appearance', theme); + await expect(page.locator('#workspace-main #timeout')).toBeEnabled(); + await page.mouse.move(0, 0); + return page.evaluate(measureAppearance, '#workspace-main'); +} + +for (const theme of THEMES) for (const [width, height] of SIZES) { + test(`appearance matches the concept ${theme} ${width}x${height}`, async ({ page, request }, testInfo) => { + await page.setViewportSize({ width, height }); + const { settings } = await serverJson(request, '/prompt_manager/settings'); + const concept = await conceptAppearance(page, theme); + await page.screenshot({ path: testInfo.outputPath(`concept-appearance-${theme}-${width}.png`), fullPage: true }); + const production = await productionAppearance(page, theme); + await page.screenshot({ path: testInfo.outputPath(`production-appearance-${theme}-${width}.png`), fullPage: true }); + expect(diffInventory(mapAppearanceConcept(concept, settings), production)).toEqual([]); + expect(production.pageScroll).toBe(false); + }); +} + +test('appearance comparison detects mutations', async ({ page, request }) => { + await page.setViewportSize({ width: 1600, height: 1100 }); + const { settings } = await serverJson(request, '/prompt_manager/settings'); + const concept = await conceptAppearance(page, 'dark'); + const expected = mapAppearanceConcept(concept, settings); + const production = await productionAppearance(page, 'dark'); + expect(diffInventory(expected, production)).toEqual([]); + // Unmapped, the concept differs only where D97 and the real timeout are recorded. + expect(diffInventory(concept, production).map(item => item.path.split('.').slice(0, 2).join('.')) + .filter((path, index, all) => all.indexOf(path) === index).sort()) + .toEqual(['operations.0', 'operations.length', 'rows.11', 'rows.12', 'rows.13', 'rows.length', 'surface.tree'].sort()); + const measure = () => page.evaluate(measureAppearance, '#workspace-main'); + const paths = async () => diffInventory(expected, await measure()).map(item => item.path); + // A removed row. + const undoRow = await mutate(page, { remove: ['#workspace-main .setting-row:nth-of-type(3)'] }); + expect(await paths()).toEqual(expect.arrayContaining(['rows.length', 'surface.tree.length'])); + await undoRow(); + // A changed label and a changed option. + await page.evaluate(() => { document.querySelector('#workspace-main label[for="density"]').textContent = 'Thumbnail sizes'; }); + expect(await paths()).toEqual(['rows.1.text.0.1']); + await page.evaluate(() => { document.querySelector('#workspace-main label[for="density"]').textContent = 'Thumbnail size'; }); + await page.evaluate(() => { document.querySelector('#workspace-main #image-quality option').textContent = 'Medium'; }); + expect(await paths()).toEqual(expect.arrayContaining(['rows.5.control.options.0'])); + await page.evaluate(() => { document.querySelector('#workspace-main #image-quality option').textContent = 'Optimized'; }); + // A swapped token. + const undoToken = await mutate(page, { css: '#workspace-main .settings-layout { --line: rgb(255, 0, 0); }' }); + expect(await paths()).toEqual(expect.arrayContaining(['row.borderTopColor'])); + await undoToken(); + expect(diffInventory(expected, await measure())).toEqual([]); +}); diff --git a/tests/browser/workspace-activity.spec.mjs b/tests/browser/workspace-activity.spec.mjs index a3a1616..72e95f2 100644 --- a/tests/browser/workspace-activity.spec.mjs +++ b/tests/browser/workspace-activity.spec.mjs @@ -284,6 +284,19 @@ test('reload during a scan reattaches to the running job', async ({ page, reques await expect(pill(page)).toBeHidden(); }); +test('an empty Activity says no job has run, and a list that cannot be read says so', async ({ page }) => { + // The empty and failed states were only reached in passing by other tests, so their coverage depended on timing. + const empty = main(page).locator('section.surface > p.empty-state'); + await page.goto(`${workspace}#/activity`); + await expect(empty).toHaveText('No jobs have run since ComfyUI started. Scans and other long operations appear here.'); + await expect(empty).not.toHaveAttribute('role'); + await expect(items(page)).toHaveCount(0); + await page.route(url => url.pathname === '/prompt_manager/jobs', route => route.abort()); + await page.reload(); + await expect(empty).toHaveText('Activity could not be read. It is tried again shortly.'); + await expect(empty).toHaveAttribute('role', 'alert'); +}); + test('lost server clears the pill and marks jobs ended', async ({ page, request }) => { // A poll interval after the restart, then failed re-reads backing off for 1 + 2 + 4 s. test.setTimeout(90_000); diff --git a/tests/browser/workspace-system.spec.mjs b/tests/browser/workspace-system.spec.mjs index b2c702d..eb1bc45 100644 --- a/tests/browser/workspace-system.spec.mjs +++ b/tests/browser/workspace-system.spec.mjs @@ -11,7 +11,7 @@ const number = value => value.toLocaleString('en-US'); test.beforeEach(async ({ page, request, browserName }) => { await startCoverage(page, browserName); - await seedConcept(request); + page.manifest = await seedConcept(request); await expect(await request.post('/__test__/system/reset')).toBeOK(); await expect(await request.post('/__test__/jobs/restart')).toBeOK(); page.systemErrors = []; @@ -255,3 +255,368 @@ test('out-of-order overview responses never repaint a newer section', async ({ p await expect(layout.locator('.summary-metrics')).toHaveCount(1); await expect(layout.locator('.two-col')).toHaveCount(1); }); + +// System › Appearance (#249 System S3 Task 8): the browser display defaults (`gallerySettings`, preferences.mjs) and the +// two host fields behind the one Save row, and the consumers that read them on their next visit. +const SETTINGS_URL = '/prompt_manager/settings'; +const INSPECT_PATH = '/prompt_manager/images/inspect/'; +const statusOf = page => page.locator('#workspace-main #settings-status'); +const saveOf = page => page.locator('#workspace-main .save-row > button'); +const storedSettings = page => page.evaluate(() => JSON.parse(localStorage.getItem('gallerySettings') || 'null')); +const imageCards = page => page.locator('#workspace-main article.media-card'); +const panel = page => page.getByRole('complementary', { name: 'Selected image details', exact: true }); + +async function openAppearance(page) { + await page.goto(`${workspace}#/system/appearance`); + await expect(page.locator('#workspace-main .settings-layout > section.surface > h2')).toHaveText('Browsing & display'); + // The host rows are enabled once the server settings answered. + await expect(page.locator('#timeout')).toBeEnabled(); +} + +async function saveAppearance(page, text) { + await expect(saveOf(page)).not.toHaveAttribute('aria-disabled', 'true'); + await saveOf(page).click(); + await expect(statusOf(page)).toHaveText(text); +} + +/** Set toggles (by row id) through their Toolkit switches. */ +async function toggle(page, values) { + for (const [id, checked] of Object.entries(values)) await page.locator(`#${id}`).setChecked(checked, { force: true }); +} + +/** + * A request barrier: one request issued and answered after everything the page has asked for so far, so a read the + * last action started has been seen (Playwright reports a page's requests in order). + */ +async function requestBarrier(page) { + expect(await page.evaluate(url => fetch(url, { cache: 'no-store' }).then(response => response.ok), SETTINGS_URL)).toBe(true); +} + +async function openImages(page) { + await page.goto(`${workspace}#/images`); + await expect(imageCards(page).first()).toBeVisible(); +} + +test('defaults apply to a new visit but not to the current view or a saved view', async ({ page, request }) => { + const created = await request.post('/prompt_manager/saved-views', { data: { page: 'images', name: 'Everything', + state: { q: '', category: '', sort: 'newest', source: 'all', folder: '', media: 'all', tags: [], favorites: false } } }); + expect(created.status()).toBe(201); + const view = (await created.json()).view; + try { + await openImages(page); + // The current view: the toolbar sets List. + await page.getByRole('group', { name: 'Image layout' }).getByRole('button', { name: 'List', exact: true }).click(); + await expect(page.locator('.image-grid')).toHaveAttribute('data-layout', 'list'); + await openAppearance(page); + await expect(page.locator('#default-view')).toHaveValue('grid'); + await page.locator('#default-view').selectOption('masonry'); + await page.locator('#density').selectOption('compact'); + await expect(statusOf(page)).toHaveText('Unsaved changes.'); + await saveAppearance(page, 'Saved in this browser.'); + const stored = await storedSettings(page); + expect([stored.workspaceLayout, stored.workspaceDensity]).toEqual(['masonry', 'compact']); + // Back in Images (the page session's last visit): the current view keeps its own layout and size. + await page.locator('#primary-nav a[data-route="images"]').click(); + await expect(page.locator('.image-grid')).toHaveAttribute('data-layout', 'list'); + await expect(page.locator('.image-grid')).toHaveAttribute('data-density', 'comfortable'); + // The saved view opens as before. + await page.goto(`${workspace}#/saved/${view.id}`); + await expect(imageCards(page).first()).toBeVisible(); + await expect(page.locator('.image-grid')).toHaveAttribute('data-layout', 'grid'); + await expect(page.locator('.image-grid')).toHaveAttribute('data-density', 'comfortable'); + // A new visit starts from the defaults. + await openImages(page); + await page.reload(); + await expect(imageCards(page).first()).toBeVisible(); + await expect(page.locator('.image-grid')).toHaveAttribute('data-layout', 'masonry'); + await expect(page.locator('.image-grid')).toHaveAttribute('data-density', 'compact'); + await expect(page.getByRole('group', { name: 'Image layout' }).getByRole('button', { name: 'Masonry', exact: true })) + .toHaveAttribute('aria-pressed', 'true'); + } finally { + await request.delete(`/prompt_manager/saved-views/${view.id}`); + } +}); + +test('the defaults reach the Images page only; embedded galleries keep their own presentation', async ({ page }) => { + await openAppearance(page); + await page.locator('#default-view').selectOption('list'); + await page.locator('#density').selectOption('compact'); + await saveAppearance(page, 'Saved in this browser.'); + // The prompt history's shared gallery has no toolbar to change them, so it opens as it always did. + await page.goto(`${workspace}#/library/${page.manifest.prompts[0].positive_id}`); + await expect(imageCards(page).first()).toBeVisible(); + await expect(page.locator('#workspace-main .image-grid')).toHaveAttribute('data-layout', 'grid'); + await expect(page.locator('#workspace-main .image-grid')).toHaveAttribute('data-density', 'comfortable'); + // The Images page starts from them. + await page.locator('#primary-nav a[data-route="images"]').click(); + await expect(imageCards(page).first()).toBeVisible(); + await expect(page.locator('.image-grid')).toHaveAttribute('data-layout', 'list'); + await expect(page.locator('.image-grid')).toHaveAttribute('data-density', 'compact'); +}); + +test('metadata off shows an explicit load and loads once on request', async ({ page }) => { + await openAppearance(page); + await toggle(page, { 'metadata-auto': false }); + await saveAppearance(page, 'Saved in this browser.'); + const reads = []; + page.on('request', item => { if (new URL(item.url()).pathname.startsWith(INSPECT_PATH)) reads.push(item.url()); }); + await openImages(page); + const first = imageCards(page).first(); + await first.locator('.image-card-actions button').first().click(); + await expect(panel(page).getByRole('status')).toHaveText('Metadata loads when you ask for it.'); + const load = panel(page).getByRole('button', { name: 'Load metadata', exact: true }); + await expect(load).toBeVisible(); + expect(reads).toEqual([]); + await load.click(); + await expect(panel(page).locator('.panel-heading h2')).toBeVisible(); + expect(reads).toHaveLength(1); + // The same file again reads nothing more (the cache is on). + await first.locator('.image-card-actions button').first().click(); + await expect(panel(page).locator('.panel-heading h2')).toBeVisible(); + await requestBarrier(page); + expect(reads).toHaveLength(1); + // The viewer's details use the same inspector: another file waits there too, and loads on request. + await imageCards(page).nth(1).locator('.image-open').click(); + const viewerDetails = page.locator('#viewer-details'); + await expect(viewerDetails.getByRole('status')).toHaveText('Metadata loads when you ask for it.'); + expect(reads).toHaveLength(1); + await viewerDetails.getByRole('button', { name: 'Load metadata', exact: true }).click(); + await expect(viewerDetails.locator('.panel-heading h2')).toBeVisible(); + expect(reads).toHaveLength(2); +}); + +test('cache off rereads the current selection; on reuses the one current record', async ({ page }) => { + const reads = []; + page.on('request', item => { if (new URL(item.url()).pathname.startsWith(INSPECT_PATH)) reads.push(item.url()); }); + for (const cache of [true, false]) { + if (!cache) { + await openAppearance(page); + await toggle(page, { 'metadata-cache': false }); + await saveAppearance(page, 'Saved in this browser.'); + await page.reload(); + } + await openImages(page); + const info = imageCards(page).first().locator('.image-card-actions button').first(); + await info.click(); + await expect(panel(page).locator('.panel-heading h2')).toBeVisible(); + await expect.poll(() => reads.length).toBeGreaterThan(0); + await requestBarrier(page); + reads.length = 0; + // Inspect the selected file twice more. + for (const _ of [1, 2]) { + const answered = cache ? null : page.waitForResponse(response => new URL(response.url()).pathname.startsWith(INSPECT_PATH)); + await info.click(); + if (answered) await answered; + await expect(panel(page).locator('.panel-heading h2')).toBeVisible(); + } + await requestBarrier(page); + expect(reads, `cache ${cache}`).toHaveLength(cache ? 0 : 2); + } +}); + +test('paths hidden only changes display', async ({ page }) => { + const rows = async () => page.locator('#viewer-details details').filter({ hasText: 'File & provenance' }).locator('dt') + .evaluateAll(terms => terms.map(term => term.textContent)); + const openViewer = async () => { + await openImages(page); + await imageCards(page).first().locator('.image-open').click(); + const provenance = page.locator('#viewer-details details').filter({ hasText: 'File & provenance' }); + await provenance.locator('summary').click(); + await expect(provenance.locator('dl')).toBeVisible(); + return provenance; + }; + let provenance = await openViewer(); + expect(await rows()).toContain('Path'); + const before = { show: await provenance.getByRole('button', { name: /^Show .* in folder$/ }).count(), + download: await provenance.getByRole('link', { name: 'Download original', exact: true }).getAttribute('href') }; + await page.keyboard.press('Escape'); + await openAppearance(page); + await toggle(page, { paths: false }); + await saveAppearance(page, 'Saved in this browser.'); + provenance = await openViewer(); + const shown = await rows(); + expect(shown).not.toContain('Path'); + expect(shown).toEqual(expect.arrayContaining(['Source', 'Size'])); + // Everything else is as it was: the folder action and the original download stay. + expect(await provenance.getByRole('button', { name: /^Show .* in folder$/ }).count()).toBe(before.show); + await expect(provenance.getByRole('link', { name: 'Download original', exact: true })).toHaveAttribute('href', before.download); +}); + +test('video defaults reach the viewer', async ({ page }) => { + await openAppearance(page); + await toggle(page, { 'video-auto': true, 'video-muted': false, 'video-loop': false }); + await saveAppearance(page, 'Saved in this browser.'); + expect(await page.evaluate(async () => (await import('/prompt_manager/js/workspace/media-viewer.mjs')).readViewerPreferences())) + .toEqual({ videoAutoplay: true, videoMute: false, videoLoop: false }); + const stored = await storedSettings(page); + expect([stored.videoAutoplay, stored.videoMute, stored.videoLoop]).toEqual([true, false, false]); +}); + +test('host fields round-trip through /settings with revision', async ({ page, request }) => { + const posts = []; + page.on('request', item => { if (item.method() === 'POST' && new URL(item.url()).pathname === SETTINGS_URL) posts.push(item.postDataJSON()); }); + const before = await serverJson(request, SETTINGS_URL); + await openAppearance(page); + await expect(page.locator('#open-mode')).toHaveValue(before.settings.webui_display_mode); + await expect(page.locator('#timeout')).toHaveValue(String(before.settings.result_timeout)); + // Only whole seconds from 0 to 300: anything else is highlighted and never sent. + for (const bad of ['301', '-1', '2.5']) { + await page.locator('#timeout').fill(bad); + await page.locator('#timeout').press('Tab'); + await expect(page.locator('#timeout')).toHaveAttribute('aria-invalid', 'true'); + await expect(statusOf(page)).toHaveText('Some changes were not saved. Review the highlighted settings.'); + await expect(saveOf(page)).toHaveAttribute('aria-disabled', 'true'); + } + await page.locator('#timeout').fill('0'); + await page.locator('#timeout').press('Tab'); + await expect(page.locator('#timeout')).not.toHaveAttribute('aria-invalid'); + await page.locator('#open-mode').selectOption('popup'); + await saveAppearance(page, 'Saved to the server configuration.'); + expect(posts).toEqual([{ webui_display_mode: 'popup', result_timeout: 0, expected_revision: before.revision }]); + const after = await serverJson(request, SETTINGS_URL); + expect([after.settings.webui_display_mode, after.settings.result_timeout]).toEqual(['popup', 0]); + expect(after.revision).not.toBe(before.revision); + // No browser key was written for a host field. + expect(await storedSettings(page)).toBeNull(); +}); + +test('server settings that cannot be read say so in their rows', async ({ page }) => { + await page.route(url => url.pathname === SETTINGS_URL, route => (route.request().method() === 'GET' + ? route.fulfill({ status: 500, contentType: 'application/json', body: JSON.stringify({ success: false, error: 'Injected failure' }) }) + : route.continue())); + await page.goto(`${workspace}#/system/appearance`); + for (const id of ['open-mode', 'timeout']) { + const row = page.locator('#workspace-main .setting-row').filter({ has: page.locator(`#${id}`) }); + await expect(row.locator('p.small')).toHaveText('Server settings could not be read.'); + await expect(page.locator(`#${id}`)).toBeDisabled(); + } + // The browser rows are this browser's, and work as before. + await expect(page.locator('#default-view')).toBeEnabled(); + await expect(page.locator('#workspace-main .setting-row p.small')).toHaveCount(2); +}); + +test('stale revision shows the conflict status', async ({ page, request }) => { + await openAppearance(page); + // Another page saves after this one read the settings. + const current = await serverJson(request, SETTINGS_URL); + await expect(await request.post(SETTINGS_URL, { data: { result_timeout: 42, expected_revision: current.revision } })).toBeOK(); + await page.locator('#open-mode').selectOption('popup'); + await saveAppearance(page, 'Settings changed elsewhere. Reload this section to see them.'); + await expect(page.locator('#open-mode')).toHaveAttribute('aria-invalid', 'true'); + // The draft is kept and the section now shows the other page's value. + await expect(page.locator('#open-mode')).toHaveValue('popup'); + await expect(page.locator('#timeout')).toHaveValue('42'); + expect((await serverJson(request, SETTINGS_URL)).settings.webui_display_mode).toBe('newtab'); + // With the reloaded revision, Save succeeds. + await saveAppearance(page, 'Saved to the server configuration.'); + expect((await serverJson(request, SETTINGS_URL)).settings.webui_display_mode).toBe('popup'); +}); + +test('reload keeps saved values; a new tab reads them', async ({ page, context }) => { + await openAppearance(page); + await page.locator('#image-quality').selectOption('high'); + await toggle(page, { 'card-details': false }); + await page.locator('#open-mode').selectOption('popup'); + await saveAppearance(page, 'Saved in this browser. Saved to the server configuration.'); + await page.reload(); + await expect(page.locator('#timeout')).toBeEnabled(); + await expect(page.locator('#image-quality')).toHaveValue('high'); + await expect(page.locator('#card-details')).not.toBeChecked(); + await expect(page.locator('#open-mode')).toHaveValue('popup'); + const other = await context.newPage(); + await openAppearance(other); + await expect(other.locator('#image-quality')).toHaveValue('high'); + await expect(other.locator('#card-details')).not.toBeChecked(); + // A change saved in one tab reaches the other's clean controls; a draft it overtakes is flagged and kept. + await other.locator('#density').selectOption('large'); + await page.locator('#density').selectOption('compact'); + await saveAppearance(page, 'Saved in this browser.'); + await expect(other.locator('#density')).toHaveValue('large'); + await expect(other.locator('#settings-status')).toHaveText('Settings changed elsewhere. Reload this section to see them.'); + await expect(other.locator('#density')).toHaveAttribute('aria-invalid', 'true'); + await page.locator('#image-quality').selectOption('low'); + await saveAppearance(page, 'Saved in this browser.'); + await expect(other.locator('#image-quality')).toHaveValue('low'); + await other.close(); +}); + +test('a change another tab saved while System was closed is a conflict on return, never saved over silently', async ({ page, context }) => { + await openAppearance(page); + await page.locator('#density').selectOption('compact'); + await expect(statusOf(page)).toHaveText('Unsaved changes.'); + // Leave System for Images: the draft stays for the page session, but System no longer listens. + await page.locator('#primary-nav a[data-route="images"]').click(); + await expect(imageCards(page).first()).toBeVisible(); + const other = await context.newPage(); + await openAppearance(other); + await other.locator('#density').selectOption('large'); + await saveAppearance(other, 'Saved in this browser.'); + await other.close(); + // Back in System: the draft is kept and flagged, and nothing was written over the other tab's value. + await page.evaluate(() => { location.hash = '#/system/appearance'; }); + await expect(page.locator('#workspace-main .settings-layout > section.surface > h2')).toHaveText('Browsing & display'); + await expect(statusOf(page)).toHaveText('Settings changed elsewhere. Reload this section to see them.'); + await expect(page.locator('#density')).toHaveAttribute('aria-invalid', 'true'); + await expect(page.locator('#density')).toHaveValue('compact'); + expect((await storedSettings(page)).workspaceDensity).toBe('large'); + // Saving after the conflict was shown is an explicit choice: it wins. + await saveAppearance(page, 'Saved in this browser.'); + expect((await storedSettings(page)).workspaceDensity).toBe('compact'); + await expect(page.locator('#density')).not.toHaveAttribute('aria-invalid'); +}); + +test('a browser that refuses storage is saved for this tab only, never "Saved"', async ({ page }) => { + await openAppearance(page); + await page.evaluate(() => { + Storage.prototype.setItem = function setItem() { throw new DOMException('Quota exceeded', 'QuotaExceededError'); }; + }); + await page.locator('#default-view').selectOption('list'); + await saveAppearance(page, 'Saved for this tab only; browser storage is unavailable.'); + expect(await storedSettings(page)).toBeNull(); + // This tab's next Images visit still uses it. + await page.locator('#primary-nav a[data-route="images"]').click(); + await expect(imageCards(page).first()).toBeVisible(); + await expect(page.locator('.image-grid')).toHaveAttribute('data-layout', 'list'); +}); + +test('added rows reach their consumers', async ({ page }) => { + await openImages(page); + await expect(imageCards(page).first().locator('.media-info h3')).toBeVisible(); + await openAppearance(page); + await toggle(page, { 'card-details': false, 'viewer-details': true }); + await saveAppearance(page, 'Saved in this browser.'); + await page.reload(); + await openImages(page); + // Show image details on cards off: no title or file line; Info and Tag stay. + const card = imageCards(page).first(); + await expect(card.locator('.media-info h3')).toBeHidden(); + await expect(card.locator('.image-card-actions button')).toHaveCount(2); + // Collapse viewer details by default: the viewer opens with its details hidden behind one toggle. + await card.locator('.image-open').click(); + const details = page.locator('#viewer-details'); + await expect(page.locator('#viewer-dialog')).toBeVisible(); + await expect(details).toBeHidden(); + const show = page.locator('#viewer-toolbar').getByRole('button', { name: 'Show details', exact: true }); + await expect(show).toHaveAttribute('aria-expanded', 'false'); + await show.click(); + await expect(details).toBeVisible(); + await expect(page.locator('#viewer-toolbar').getByRole('button', { name: 'Hide details', exact: true })).toHaveAttribute('aria-expanded', 'true'); +}); + +test('reset restores defaults without touching other keys', async ({ page }) => { + await page.goto(`${workspace}#/system/appearance`); + await page.evaluate(() => localStorage.setItem('gallerySettings', JSON.stringify({ gridColumns: 5, defaultLimit: 200, + thumbnailPromptDismissed: 'kept', workspaceLayout: 'list', videoMute: false, showImageInfo: false }))); + await page.reload(); + await expect(page.locator('#timeout')).toBeEnabled(); + await expect(page.locator('#default-view')).toHaveValue('list'); + const reset = page.locator('#workspace-main .operation').filter({ hasText: 'Reset display defaults' }).getByRole('button', { name: 'Reset', exact: true }); + await reset.click(); + await expect(page.locator('#default-view')).toHaveValue('grid'); + await expect(page.locator('#video-muted')).toBeChecked(); + await expect(statusOf(page)).toHaveText('Unsaved changes.'); + await saveAppearance(page, 'Saved in this browser.'); + const stored = await storedSettings(page); + expect(stored).toEqual({ gridColumns: 5, defaultLimit: 200, thumbnailPromptDismissed: 'kept', workspaceLayout: 'grid', + videoMute: true, showImageInfo: true }); +}); diff --git a/tests/browser/workspace.spec.mjs b/tests/browser/workspace.spec.mjs index 4071ee3..4057b78 100644 --- a/tests/browser/workspace.spec.mjs +++ b/tests/browser/workspace.spec.mjs @@ -427,7 +427,7 @@ for (const width of [1600, 390]) test(`overview and handoff render with concept const SYSTEM_SECTION_LABELS = ['Overview', 'Appearance', 'Sources & scans', 'Integrations', 'Auto-tagging', 'Maintenance', 'Backup & export', 'Diagnostics & logs']; const SYSTEM_HASHES = ['overview', 'appearance', 'sources', 'integrations', 'tagging', 'maintenance', 'backups', 'diagnostics']; -const SYSTEM_HANDOFFS = { appearance: ['/prompt_manager/gallery', '/prompt_manager/admin'], sources: ['/prompt_manager/admin', '/prompt_manager/gallery'], +const SYSTEM_HANDOFFS = { sources: ['/prompt_manager/admin', '/prompt_manager/gallery'], maintenance: ['/prompt_manager/admin', '/prompt_manager/gallery'] }; test('System is native: the concept shell, section navigation, sections, hand-offs and the Save row', async ({ page }) => { @@ -464,14 +464,14 @@ test('System is native: the concept shell, section navigation, sections, hand-of await expect(nav.locator('[aria-pressed="true"]')).toHaveCount(1); // The section changes in place: focus stays on the pressed section, not the page title. await expect(button).toBeFocused(); - // S2 Task 6: Overview is native (workspace-system.spec.mjs); Maintenance shows its health-check notice above - // the hand-off (D123); every other section hands off. + // S2 Task 6: Overview is native (workspace-system.spec.mjs), and so is Appearance (S3 Task 8); Maintenance shows + // its health-check notice above the hand-off (D123); every other section hands off. if (hash === 'overview') await expect(layout.locator(':scope > .summary-metrics')).toBeVisible(); expect(await layout.evaluate(node => [...node.children].map(child => `${child.localName}.${child.className}`))) - .toEqual({ overview: ['div.summary-metrics', 'div.two-col', 'div.save-row'], + .toEqual({ overview: ['div.summary-metrics', 'div.two-col', 'div.save-row'], appearance: ['section.surface', 'div.save-row'], maintenance: ['div.notice', 'section.surface handoff', 'div.save-row'] }[hash] ?? ['section.surface handoff', 'div.save-row']); - if (hash === 'overview') continue; + if (hash === 'overview' || hash === 'appearance') continue; const handoff = layout.locator('section.handoff'); await expect(handoff.locator('h2')).toHaveText('Your current tools are ready'); expect(await handoff.locator('a.workspace-link').evaluateAll(links => links.map(link => link.getAttribute('href')))) diff --git a/tests/browser/workspace_system_fixture.py b/tests/browser/workspace_system_fixture.py index 6027c64..2413c1f 100644 --- a/tests/browser/workspace_system_fixture.py +++ b/tests/browser/workspace_system_fixture.py @@ -31,9 +31,27 @@ MAX_UNTHUMBNAILED = 50 def register_system_commands(app, routes, api, root: Path): - from py.config import GalleryConfig + # System › Appearance saves the host fields to this server's config.json (S3); + # reset restores both. The file is this server's own, inside its temp root: + # reset rewrites or deletes it, so a server whose configuration points + # anywhere else (a real config.json) refuses to register these hooks. + config_path = root / "config.json" + configured_path = os.environ.get("PROMPT_MANAGER_CONFIG_PATH") + if ( + configured_path is None + or Path(configured_path).resolve() != config_path.resolve() + ): + raise RuntimeError( + f"PROMPT_MANAGER_CONFIG_PATH must be {config_path}, not {configured_path!r}" + ) + from py.config import GalleryConfig, PromptManagerConfig configured = list(GalleryConfig.MONITORING_DIRECTORIES) + config_bytes = config_path.read_bytes() if config_path.exists() else None + host_values = ( + PromptManagerConfig.RESULT_TIMEOUT, + PromptManagerConfig.WEBUI_DISPLAY_MODE, + ) failures: dict[str, int] = {} @web.middleware @@ -52,6 +70,16 @@ def register_system_commands(app, routes, api, root: Path): failures.clear() GalleryConfig.MONITORING_DIRECTORIES = list(configured) api._health_report = None + PromptManagerConfig.RESULT_TIMEOUT, PromptManagerConfig.WEBUI_DISPLAY_MODE = ( + host_values + ) + if config_bytes is None: + config_path.unlink(missing_ok=True) + else: + # Replace in one step, as the server writes it (Windows preflight). + staged = config_path.with_name(config_path.name + ".reset") + staged.write_bytes(config_bytes) + os.replace(staged, config_path) @routes.post("/__test__/system/reset") async def system_reset(request): diff --git a/tests/js/gallery-settings.test.js b/tests/js/gallery-settings.test.js index c3681a2..a8fbf65 100644 --- a/tests/js/gallery-settings.test.js +++ b/tests/js/gallery-settings.test.js @@ -61,3 +61,50 @@ test("infinite scrolling is off by default and must be a boolean", () => { assert.equal(normalizeGallerySettings({ infiniteScroll: true }).infiniteScroll, true); assert.equal(normalizeGallerySettings({ infiniteScroll: "yes" }).infiniteScroll, false); }); + +test("the workspace keys have their own defaults and validation", () => { + const { WORKSPACE_LAYOUTS, WORKSPACE_DENSITIES } = require("../../web/js/gallery-settings.js"); + assert.equal(DEFAULT_SETTINGS.workspaceLayout, "grid"); + assert.equal(DEFAULT_SETTINGS.workspaceDensity, "comfortable"); + assert.deepEqual(WORKSPACE_LAYOUTS, ["grid", "list", "masonry"]); + assert.deepEqual(WORKSPACE_DENSITIES, ["comfortable", "large", "compact"]); + assert.equal(normalizeGallerySettings({ workspaceLayout: "masonry" }).workspaceLayout, "masonry"); + assert.equal(normalizeGallerySettings({ workspaceLayout: "mosaic" }).workspaceLayout, "grid"); + assert.equal(normalizeGallerySettings({ workspaceDensity: "large" }).workspaceDensity, "large"); + assert.equal(normalizeGallerySettings({ workspaceDensity: 3 }).workspaceDensity, "comfortable"); + // The legacy gallery's own view mode is separate: it has no masonry. + assert.equal(normalizeGallerySettings({ defaultViewMode: "masonry" }).defaultViewMode, "grid"); +}); + +test("a legacy gallery Save keeps the workspace keys and every other stored key", () => { + 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 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) }, + }); + 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() {} }); + 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); +}); diff --git a/tests/js/workspace-images-state.test.js b/tests/js/workspace-images-state.test.js index b17aa9b..bb81a6e 100644 --- a/tests/js/workspace-images-state.test.js +++ b/tests/js/workspace-images-state.test.js @@ -478,3 +478,19 @@ test('a refresh deferred behind a page load is dropped when the stream is dispos await new Promise(resolve=>setImmediate(resolve)); assert.equal(requests.length,2,'no refresh runs after dispose'); }); + +test('defaults apply to a new visit but not to the current view or a saved view', async () => { + const { initialImagesView } = await source; + assert.equal(typeof initialImagesView, 'function', 'initialImagesView is exported'); + const preferences = { workspaceLayout: 'masonry', workspaceDensity: 'compact' }; + // A new visit starts from System › Appearance. + assert.deepEqual(initialImagesView({ preferences }), { layout: 'masonry', density: 'compact' }); + // The current view (the last visit) keeps its own toolbar choices. + assert.deepEqual(initialImagesView({ prior: { layout: 'list', density: 'large' }, preferences }), { layout: 'list', density: 'large' }); + assert.deepEqual(initialImagesView({ prior: {}, preferences }), { layout: 'grid', density: 'comfortable' }); + // A saved view opens as it always did. + assert.deepEqual(initialImagesView({ saved: { q: '' }, preferences }), { layout: 'grid', density: 'comfortable' }); + // Without readable preferences nothing changes. + assert.deepEqual(initialImagesView({}), { layout: 'grid', density: 'comfortable' }); + assert.deepEqual(initialImagesView({ preferences: {} }), { layout: 'grid', density: 'comfortable' }); +}); diff --git a/tests/js/workspace-inspection-state.test.js b/tests/js/workspace-inspection-state.test.js index 82ea79d..27d095e 100644 --- a/tests/js/workspace-inspection-state.test.js +++ b/tests/js/workspace-inspection-state.test.js @@ -825,3 +825,73 @@ test('refresh re-reads the selected file and keeps the shown record until the ne assert.equal(controller.state.record.identity.path, 'folder/b.png'); controller.dispose(); assert.equal(await controller.refresh(), undefined); }); + +test('metadata off shows an explicit load and loads once on request', async () => { + const { api, requests } = pendingApi(), updates = []; + const controller = contract('createInspectionController')({ api, extractFacts: extractGenerationFacts, autoLoad: false }); + controller.subscribe(state => updates.push(state)); + // Selecting reads nothing: the state says the metadata waits for a request. + assert.equal(controller.select(image()), undefined); + assert.equal(requests.length, 0); + assert.equal(controller.state.deferred, true); + assert.equal(controller.state.loading, false); + assert.equal(controller.state.record, null); + // Selecting the same file again still reads nothing. + controller.select({ ...image() }); + assert.equal(requests.length, 0); + const load = controller.load(); + assert.equal(requests.length, 1); + assert.equal(controller.state.loading, true); + assert.equal(controller.state.deferred, false); + // A second press while it reads, and the same file selected again once it loaded, read nothing more. + controller.load(); + assert.equal(requests.length, 1); + requests[0].resolve(envelope()); await load; + assert.equal(controller.state.record.generation.seed.value, '9007199254740993'); + controller.load(); controller.select(image()); + assert.equal(requests.length, 1); + // Another file waits for its own request again. + controller.select({ ...image(), path: 'folder/b.png', filename: 'b.png', url: image().url.replace('a%20%23%25', 'b') }); + assert.equal(controller.state.deferred, true); + assert.equal(requests.length, 1); + // The default controller loads on selection, as before. + const eager = contract('createInspectionController')({ api, extractFacts: extractGenerationFacts }); + eager.select(image()); + assert.equal(requests.length, 2); + assert.equal(eager.state.deferred, false); + eager.dispose(); controller.dispose(); + assert.equal(controller.load(), undefined); +}); + +test('cache off rereads the current selection; on reuses the one current record', async () => { + for (const cache of [true, false]) { + const { api, requests } = pendingApi(); + const controller = contract('createInspectionController')({ api, extractFacts: extractGenerationFacts, cache }); + const first = controller.select(image()); + // A read in flight is shared either way. + controller.select(image()); + assert.equal(requests.length, 1); + requests[0].resolve(envelope()); await first; + const again = controller.select(image()); + assert.equal(requests.length, cache ? 1 : 2, `cache ${cache}`); + if (!cache) { requests[1].resolve(envelope()); await again; assert.equal(controller.state.record.identity.path, 'folder/a #%.png'); } + controller.dispose(); + } +}); + +test('cache off never rereads a selection whose source changed, and refresh never loads a deferred one', async () => { + const { api, requests } = pendingApi(); + const controller = contract('createInspectionController')({ api, extractFacts: extractGenerationFacts, cache: false }); + const first = controller.select(image()); + requests[0].reject(new ApiError(409, refreshMessage, 'source-changed')); await first; + assert.equal(controller.state.refreshRequired, true); + controller.select(image()); + assert.equal(requests.length, 1); + controller.dispose(); + const deferred = contract('createInspectionController')({ api, extractFacts: extractGenerationFacts, autoLoad: false }); + deferred.select(image()); + assert.equal(deferred.refresh(), undefined); + assert.equal(requests.length, 1); + assert.equal(deferred.state.deferred, true); + deferred.dispose(); +}); diff --git a/tests/js/workspace-preferences.test.js b/tests/js/workspace-preferences.test.js new file mode 100644 index 0000000..07b18ca --- /dev/null +++ b/tests/js/workspace-preferences.test.js @@ -0,0 +1,193 @@ +// Browser display defaults (#249 System S3 Task 7): the workspace reads and writes the legacy gallery's own +// `gallerySettings` object (Q3, "One display configuration"), writes only the keys it changed, never consumes the +// legacy paging keys, and says when this browser refused to keep a change. +import test from 'node:test'; +import assert from 'node:assert/strict'; +import gallerySettings from '../../web/js/gallery-settings.js'; + +const module = await import('../../web/js/workspace/preferences.mjs').catch(error => { + if (error.code !== 'ERR_MODULE_NOT_FOUND') throw error; + return {}; +}); +globalThis.GallerySettings = gallerySettings; +const KEY = 'gallerySettings'; + +/** A localStorage stand-in holding `text` under the settings key; `setItem` can be made to throw. */ +function storage(text = null, { quota = false } = {}) { + const writes = []; + const store = { + text, + writes, + getItem(key) { assert.equal(key, KEY); return store.text; }, + setItem(key, value) { + assert.equal(key, KEY); + writes.push(value); + if (quota) { const error = new Error('Quota exceeded'); error.name = 'QuotaExceededError'; throw error; } + store.text = value; + }, + removeItem() { assert.fail('Preferences never remove the shared settings'); }, + }; + return store; +} +const denied = () => { const error = new Error('Denied'); error.name = 'SecurityError'; throw error; }; + +test('normalizes new workspace keys and keeps legacy keys', () => { + assert.equal(typeof module.readPreferences, 'function', 'preferences.mjs exports readPreferences'); + module.resetTabPreferences(); + const legacy = { defaultViewMode: 'list', gridColumns: 5, defaultLimit: 200, thumbnailPromptDismissed: 'sig', extra: 1 }; + const store = storage(JSON.stringify({ ...legacy, workspaceLayout: 'masonry', workspaceDensity: 'compact', showFilePaths: false })); + const { values, persisted } = module.readPreferences(() => store); + assert.equal(persisted, true); + assert.equal(values.workspaceLayout, 'masonry'); + assert.equal(values.workspaceDensity, 'compact'); + assert.equal(values.showFilePaths, false); + // Invalid workspace values fall back to the defaults, as every legacy key does. + store.text = JSON.stringify({ workspaceLayout: 'mosaic', workspaceDensity: 7, videoMute: 'no' }); + const fallback = module.readPreferences(() => store).values; + assert.equal(fallback.workspaceLayout, 'grid'); + assert.equal(fallback.workspaceDensity, 'comfortable'); + assert.equal(fallback.videoMute, true); + // The legacy normalizer knows the new keys and still keeps unknown ones. + const normalized = gallerySettings.normalizeGallerySettings({ ...legacy, workspaceLayout: 'list' }); + assert.equal(normalized.workspaceLayout, 'list'); + assert.equal(normalized.workspaceDensity, 'comfortable'); + assert.equal(normalized.extra, 1); + assert.equal(normalized.thumbnailPromptDismissed, 'sig'); + assert.deepEqual(gallerySettings.WORKSPACE_LAYOUTS, ['grid', 'list', 'masonry']); + assert.deepEqual(gallerySettings.WORKSPACE_DENSITIES, ['comfortable', 'large', 'compact']); +}); + +test('write touches only dirty keys', () => { + module.resetTabPreferences(); + const stored = { defaultViewMode: 'list', gridColumns: 5, thumbnailsGenerated: true, extra: { kept: true }, videoMute: false }; + const store = storage(JSON.stringify(stored)); + const result = module.writePreferences({ workspaceLayout: 'masonry', showImageInfo: false }, () => store); + assert.deepEqual(result, { persisted: true, conflicts: [] }); + assert.equal(store.writes.length, 1); + // Every other key, legacy or unknown, is written back exactly as it was stored: no defaults are filled in. + assert.deepEqual(JSON.parse(store.text), { ...stored, workspaceLayout: 'masonry', showImageInfo: false }); + // It re-reads the stored object, so a key another page wrote after this one read is kept. + store.text = JSON.stringify({ ...JSON.parse(store.text), gridColumns: 9 }); + module.writePreferences({ videoLoop: false }, () => store); + assert.equal(JSON.parse(store.text).gridColumns, 9); + assert.equal(JSON.parse(store.text).workspaceLayout, 'masonry'); + // An empty or malformed stored value starts from an empty object, never the defaults. + for (const text of [null, '{broken', '[]', '42']) { + const fresh = storage(text); + assert.equal(module.writePreferences({ cacheMetadata: false }, () => fresh).persisted, true); + assert.deepEqual(JSON.parse(fresh.text), { cacheMetadata: false }); + } + // Only workspace keys with valid values are accepted; nothing is written for a bad change. + for (const bad of [{ gridColumns: 4 }, { defaultLimit: 50 }, { workspaceLayout: 'mosaic' }, { videoMute: 'yes' }, { imageQuality: 'ultra' }]) { + const untouched = storage(JSON.stringify(stored)); + assert.throws(() => module.writePreferences(bad, () => untouched), TypeError); + assert.equal(untouched.writes.length, 0); + } + // A key changed elsewhere since this tab read it (its baseline) is reported, and the explicit save still wins. + const elsewhere = storage(JSON.stringify({ videoLoop: false })); + const outcome = module.writePreferences({ videoLoop: true, videoMute: false }, () => elsewhere, { baseline: { videoLoop: true, videoMute: true } }); + assert.deepEqual(outcome, { persisted: true, conflicts: ['videoLoop'] }); + assert.equal(JSON.parse(elsewhere.text).videoLoop, true); +}); + +test('denied storage reports tab-only', () => { + module.resetTabPreferences(); + const read = module.readPreferences(denied); + assert.equal(read.persisted, false); + assert.equal(read.values.workspaceLayout, 'grid'); + const result = module.writePreferences({ workspaceLayout: 'list', videoAutoplay: true }, denied); + assert.deepEqual(result, { persisted: false, conflicts: [] }); + // The change is kept for this tab: every reader in it (a new Images visit, the viewer) sees it. + const tab = module.readPreferences(denied); + assert.equal(tab.persisted, false); + assert.equal(tab.values.workspaceLayout, 'list'); + assert.equal(tab.values.videoAutoplay, true); + // A getItem that throws is denied storage too. + const throwing = { getItem: denied, setItem: denied }; + assert.equal(module.readPreferences(() => throwing).values.workspaceLayout, 'list'); + assert.equal(module.writePreferences({ workspaceDensity: 'large' }, () => throwing).persisted, false); + // Once storage accepts a write, the tab copy of that key gives way to the stored value. + const working = storage(JSON.stringify({ workspaceLayout: 'masonry' })); + assert.equal(module.writePreferences({ workspaceLayout: 'masonry' }, () => working).persisted, true); + assert.equal(module.readPreferences(() => working).values.workspaceLayout, 'masonry'); + module.resetTabPreferences(); + assert.equal(module.readPreferences(denied).values.videoAutoplay, false); +}); + +test('quota error keeps previous bytes', () => { + module.resetTabPreferences(); + const before = JSON.stringify({ gridColumns: 6, workspaceDensity: 'large' }); + const store = storage(before, { quota: true }); + const result = module.writePreferences({ workspaceDensity: 'compact' }, () => store); + assert.deepEqual(result, { persisted: false, conflicts: [] }); + assert.equal(store.writes.length, 1); + assert.equal(store.text, before); + // This tab keeps the change it could not store. + assert.equal(module.readPreferences(() => store).values.workspaceDensity, 'compact'); + module.resetTabPreferences(); + assert.equal(module.readPreferences(() => store).values.workspaceDensity, 'large'); +}); + +/** A window stand-in that records its storage listeners. */ +function fakeWindow() { + const listeners = new Set(); + return { + listeners, + addEventListener(type, listener) { assert.equal(type, 'storage'); listeners.add(listener); }, + removeEventListener(type, listener) { assert.equal(type, 'storage'); listeners.delete(listener); }, + fire(event) { for (const listener of [...listeners]) listener(event); }, + }; +} + +test('storage event updates clean values and flags dirty conflicts', () => { + module.resetTabPreferences(); + const store = storage(JSON.stringify({ workspaceLayout: 'list', videoMute: false })); + const win = fakeWindow(); + const seen = []; + const unsubscribe = module.subscribePreferences(win, next => seen.push(next), () => store); + assert.equal(win.listeners.size, 1); + // Another key, or another storage area's event, is not ours. + win.fire({ key: 'pm_update_notice_dismissed', storageArea: store }); + win.fire({ key: KEY, storageArea: { other: true } }); + assert.equal(seen.length, 0); + win.fire({ key: KEY, storageArea: store }); + win.fire({ key: null, storageArea: store }); // localStorage.clear() elsewhere + assert.equal(seen.length, 2); + assert.equal(seen[0].values.workspaceLayout, 'list'); + unsubscribe(); + assert.equal(win.listeners.size, 0); + // The reconciliation of the System drafts: clean values follow the other page; a key this tab has a draft for and + // the other page changed is a conflict, and the draft stays. + const baseline = { workspaceLayout: 'grid', videoMute: true, videoLoop: true }; + const next = { workspaceLayout: 'list', videoMute: true, videoLoop: false }; + assert.deepEqual(module.reconcilePreferences({ baseline, drafts: { videoMute: false, videoLoop: false }, next }), { conflicts: [] }); + assert.deepEqual(module.reconcilePreferences({ baseline, drafts: { workspaceLayout: 'masonry' }, next }), { conflicts: ['workspaceLayout'] }); + assert.deepEqual(module.reconcilePreferences({ baseline, drafts: { workspaceLayout: 'list' }, next }), { conflicts: [] }); +}); + +test('legacy paging keys are never consumed by the workspace', () => { + module.resetTabPreferences(); + const store = storage(JSON.stringify({ defaultLimit: 500, gridColumns: 12, infiniteScroll: true, lazyLoading: false, + defaultViewMode: 'list', debugMode: true, apiTimeout: 9, thumbnailsGenerated: true, checkThumbnailsAtStartup: false })); + const { values } = module.readPreferences(() => store); + assert.deepEqual(Object.keys(values).sort(), [...module.WORKSPACE_PREFERENCE_KEYS].sort()); + for (const key of ['defaultLimit', 'gridColumns', 'infiniteScroll', 'lazyLoading', 'defaultViewMode', 'debugMode', 'apiTimeout', + 'thumbnailsGenerated', 'checkThumbnailsAtStartup', 'thumbnailPromptDismissed']) assert.equal(Object.hasOwn(values, key), false, key); + // The display defaults are the gallery's own defaults for these keys. + const defaults = module.preferenceDefaults(); + for (const key of module.WORKSPACE_PREFERENCE_KEYS) assert.equal(defaults[key], gallerySettings.DEFAULT_SETTINGS[key], key); +}); + +test('a later save flushes the changes kept for this tab only', () => { + module.resetTabPreferences(); + const store = storage(JSON.stringify({ gridColumns: 4 }), { quota: true }); + assert.equal(module.writePreferences({ workspaceLayout: 'list' }, () => store).persisted, false); + // Storage accepts writes again: saving another key stores the tab-only change with it, so "Saved" is true for both. + const working = storage(store.text); + assert.deepEqual(module.writePreferences({ videoLoop: false }, () => working), { persisted: true, conflicts: [] }); + assert.deepEqual(JSON.parse(working.text), { gridColumns: 4, workspaceLayout: 'list', videoLoop: false }); + // Nothing is overlaid any more: another page's later change reads through. + working.text = JSON.stringify({ ...JSON.parse(working.text), workspaceLayout: 'masonry' }); + assert.equal(module.readPreferences(() => working).values.workspaceLayout, 'masonry'); + module.resetTabPreferences(); +}); diff --git a/tests/js/workspace-system-appearance.test.js b/tests/js/workspace-system-appearance.test.js new file mode 100644 index 0000000..497df7f --- /dev/null +++ b/tests/js/workspace-system-appearance.test.js @@ -0,0 +1,188 @@ +// System › Appearance (#249 System S3 Task 8): the concept's "Browsing & display" rows (app.js 676–678) on the drafts +// of the one Save row, their stored values (Q3 mappings), the host timeout's 0–300 whole seconds and the D97 rows. +const test = require('node:test'); +const assert = require('node:assert/strict'); +const gallerySettings = require('../../web/js/gallery-settings.js'); + +const appearanceModule = import('../../web/js/workspace/system-appearance.mjs'); +const stateModule = import('../../web/js/workspace/system-state.mjs'); +const coreModule = import('../../web/js/shared/toolkit/core.js'); +globalThis.GallerySettings = gallerySettings; + +/** A minimal DOM whose elements count their live listeners (a listener's signal aborts it, as in a browser). */ +function fakeDOM() { + const previous = global.document; + const elements = []; + class Element { + constructor(tag) { + this.tagName = tag.toUpperCase(); + this.children = []; + this.attributes = {}; + this.listeners = []; + this.dataset = {}; + this.className = ''; + this.id = ''; + this.value = ''; + this._text = ''; + elements.push(this); + } + get textContent() { return this._text + this.children.map(child => child.textContent).join(''); } + set textContent(text) { this.children = []; this._text = String(text); } + append(...nodes) { + for (const node of nodes) { + const child = typeof node === 'string' ? Object.assign(new Element('#text'), { _text: node }) : node; + child.parentNode = this; + this.children.push(child); + } + } + remove() { if (this.parentNode) this.parentNode.children = this.parentNode.children.filter(child => child !== this); this.parentNode = null; } + replaceChildren(...nodes) { this.children = []; this._text = ''; this.append(...nodes); } + setAttribute(name, value) { this.attributes[name] = String(value); } + getAttribute(name) { return Object.hasOwn(this.attributes, name) ? this.attributes[name] : null; } + removeAttribute(name) { delete this.attributes[name]; } + addEventListener(type, callback, options = {}) { + if (options.signal?.aborted) return; + const entry = { type, callback }; + this.listeners.push(entry); + options.signal?.addEventListener('abort', () => { this.listeners = this.listeners.filter(item => item !== entry); }); + } + removeEventListener(type, callback) { this.listeners = this.listeners.filter(item => item.type !== type || item.callback !== callback); } + dispatch(type) { for (const { callback } of this.listeners.filter(item => item.type === type)) callback({ type, target: this }); } + click() { this.dispatch('click'); } + } + global.document = { + createElement: tag => new Element(tag), + createTextNode: text => Object.assign(new Element('#text'), { _text: String(text) }), + }; + const walk = (node, found = []) => { found.push(node); node.children.forEach(child => walk(child, found)); return found; }; + return { + listenerCount: () => elements.reduce((sum, element) => sum + element.listeners.length, 0), + find: (root, predicate) => walk(root).find(predicate), + tags: node => node.children.map(child => child.tagName.toLowerCase()), + restore() { global.document = previous; }, + }; +} + +const BROWSER = { workspaceLayout: 'grid', workspaceDensity: 'comfortable', autoLoadMetadata: true, cacheMetadata: true, + imageQuality: 'medium', videoAutoplay: false, videoMute: true, videoLoop: true, showFilePaths: true, showImageInfo: true, + sidebarCollapsedByDefault: false }; + +async function mounted({ host = { webui_display_mode: 'newtab', result_timeout: 5 }, browser = BROWSER, hostFails = false } = {}) { + const { mountAppearance } = await appearanceModule; + const { createSystemDrafts } = await stateModule; + const { createScope } = await coreModule; + const dom = fakeDOM(); + let hostValues = null; + const drafts = createSystemDrafts({ + readBrowser: () => ({ ...browser }), readRuntime: () => ({}), + readHost: async () => { if (hostFails) throw new Error('offline'); return { values: hostValues, revision: 'sha256:one' }; }, + writeBrowser: async () => ({ persisted: true }), writeHost: async () => ({ success: true, persisted: true }), writeRuntime: async () => ({}), + }); + await drafts.load('browser'); + const scope = createScope(); + const [surface] = mountAppearance(null, { scope, drafts }); + const byId = id => dom.find(surface, node => node.dataset?.setting === id); + return { dom, drafts, scope, surface, byId, loadHost: async () => { hostValues = host; await drafts.load('host'); }, + fixHost: () => { hostFails = false; } }; +} + +test('the surface is the concept section with the concept rows in order, then the D97 rows', async () => { + const { APPEARANCE_ROWS } = await appearanceModule; + const { dom, surface, scope } = await mounted(); + try { + assert.equal(surface.className, 'surface'); + assert.equal(surface.children[0].textContent, 'Browsing & display'); + assert.equal(surface.children[1].textContent, 'One set of defaults; toolbar changes apply to the current view.'); + const rows = surface.children.slice(2); + const titles = rows.map(row => row.children[0].children[0].textContent); + assert.deepEqual(titles, ['Default layout', 'Thumbnail size', 'Continuous browsing', 'Load metadata on selection', + 'Cache metadata in this browser', 'Image loading quality', 'Autoplay videos', 'Mute videos by default', 'Loop videos', + 'Show file paths', 'Open from ComfyUI', 'Node search auto-hide (seconds)', 'Show image details on cards', + 'Collapse viewer details by default', 'Reset display defaults']); + const continuous = rows[2]; + assert.deepEqual(dom.tags(continuous.children[0]), ['h3', 'p']); + assert.equal(continuous.children[1].className, 'chip blue'); + assert.equal(continuous.children[1].textContent, 'Infinite scroll'); + assert.equal(rows.at(-1).className, 'operation'); + assert.equal(APPEARANCE_ROWS.filter(Array.isArray).length, 13); + } finally { scope.dispose(); assert.equal(dom.listenerCount(), 0); dom.restore(); } +}); + +test('added rows reach their consumers: each control edits its stored key with the Q3 values', async () => { + const { dom, drafts, scope, byId, loadHost } = await mounted(); + try { + // Host rows stay disabled until the server settings answered. + assert.equal(byId('open-mode').disabled, true); + assert.equal(byId('timeout').disabled, true); + await loadHost(); + assert.equal(byId('open-mode').disabled, false); + assert.equal(byId('timeout').value, '5'); + assert.deepEqual(byId('image-quality').children.map(option => [option.value, option.textContent]), + [['medium', 'Optimized'], ['high', 'Original'], ['low', 'Fast']]); + assert.deepEqual(byId('open-mode').children.map(option => [option.value, option.textContent]), + [['newtab', 'New tab'], ['popup', 'Popup window']]); + const edits = [['default-view', 'masonry'], ['density', 'large'], ['image-quality', 'low'], ['open-mode', 'popup']]; + for (const [id, value] of edits) { byId(id).value = value; byId(id).dispatch('change'); } + for (const id of ['metadata-auto', 'metadata-cache', 'video-muted', 'video-loop', 'paths', 'card-details']) { + byId(id).checked = false; byId(id).dispatch('change'); + } + for (const id of ['video-auto', 'viewer-details']) { byId(id).checked = true; byId(id).dispatch('change'); } + assert.deepEqual(drafts.changes('browser'), { workspaceLayout: 'masonry', workspaceDensity: 'large', imageQuality: 'low', + autoLoadMetadata: false, cacheMetadata: false, videoMute: false, videoLoop: false, showFilePaths: false, showImageInfo: false, + videoAutoplay: true, sidebarCollapsedByDefault: true }); + assert.deepEqual(drafts.changes('host'), { webui_display_mode: 'popup' }); + } finally { scope.dispose(); dom.restore(); } +}); + +test('the auto-hide timeout takes whole seconds from 0 to 300 only', async () => { + const { resultTimeout } = await appearanceModule; + assert.deepEqual(['0', '30', ' 300 ', '301', '-1', '2.5', '', 'abc', '1e2'].map(resultTimeout), [0, 30, 300, null, null, null, null, null, null]); + const { dom, drafts, scope, byId, loadHost } = await mounted(); + try { + await loadHost(); + const timeout = byId('timeout'); + timeout.value = '45'; timeout.dispatch('change'); + assert.deepEqual(drafts.changes('host'), { result_timeout: 45 }); + timeout.value = '301'; timeout.dispatch('change'); + assert.deepEqual(drafts.changes('host'), {}); + assert.deepEqual([...drafts.failed('host')], ['result_timeout']); + timeout.value = '0'; timeout.dispatch('change'); + assert.deepEqual(drafts.changes('host'), { result_timeout: 0 }); + assert.deepEqual([...drafts.failed('host')], []); + } finally { scope.dispose(); dom.restore(); } +}); + +test('reset restores defaults without touching other keys', async () => { + const { DISPLAY_KEYS } = await appearanceModule; + const changed = { ...BROWSER, workspaceLayout: 'list', videoMute: false, showImageInfo: false, sidebarCollapsedByDefault: true }; + const { dom, drafts, scope, surface, loadHost } = await mounted({ browser: changed }); + try { + await loadHost(); + drafts.set('host', 'result_timeout', 9); + const reset = dom.find(surface.children.at(-1), node => node.tagName === 'BUTTON'); + assert.equal(reset.textContent, 'Reset'); + reset.click(); + assert.deepEqual(drafts.changes('browser'), { workspaceLayout: 'grid', videoMute: true, showImageInfo: true, sidebarCollapsedByDefault: false }); + // The host fields and every key outside the display defaults are left as they are. + assert.deepEqual(drafts.changes('host'), { result_timeout: 9 }); + assert.deepEqual([...DISPLAY_KEYS].sort(), Object.keys(BROWSER).sort()); + } finally { scope.dispose(); dom.restore(); } +}); + +test('host rows that could not be read stay disabled and say so; a later read clears it', async () => { + const { HOST_UNAVAILABLE } = await appearanceModule; + const { dom, drafts, scope, byId, surface, fixHost } = await mounted({ hostFails: true }); + const notes = () => surface.children.slice(2).flatMap(row => row.children[0].children.slice(2).map(note => [row.children[0].children[0].textContent, note.className, note.textContent])); + try { + assert.deepEqual(notes(), []); + assert.equal(await drafts.reloadHost(), false); + assert.deepEqual(notes(), [['Open from ComfyUI', 'small', HOST_UNAVAILABLE], ['Node search auto-hide (seconds)', 'small', HOST_UNAVAILABLE]]); + assert.equal(byId('timeout').disabled, true); + // Repaints keep one line per row. + drafts.set('browser', 'videoMute', false); + assert.equal(notes().length, 2); + fixHost(); + assert.equal(await drafts.reloadHost(), true); + assert.deepEqual(notes(), []); + } finally { scope.dispose(); assert.equal(dom.listenerCount(), 0); dom.restore(); } +}); diff --git a/tests/js/workspace-system-state.test.js b/tests/js/workspace-system-state.test.js index a9e0911..af60efb 100644 --- a/tests/js/workspace-system-state.test.js +++ b/tests/js/workspace-system-state.test.js @@ -259,3 +259,60 @@ test('a failed read keeps the store usable and drafts survive a reload (Q10)', a assert.equal(failing.dirty(), false); assert.throws(() => createSystemDrafts({ ...domains, writeHost: null }), TypeError); }); + +test('a rejected value drops its draft and is highlighted; a conflict elsewhere keeps the drafts', async () => { + const domains = adapters({ browser: { videoMute: true, videoLoop: true }, host: { result_timeout: 5 } }); + const drafts = await loaded(domains); + assert.deepEqual(drafts.saved('browser'), { videoMute: true, videoLoop: true }); + drafts.set('host', 'result_timeout', 12); + assert.equal(drafts.dirty('host'), true); + drafts.reject('host', 'result_timeout'); + assert.equal(drafts.dirty('host'), false); + assert.deepEqual([...drafts.failed('host')], ['result_timeout']); + assert.equal(drafts.get('host', 'result_timeout'), 5); + // A valid value clears the highlight. + drafts.set('host', 'result_timeout', 7); + assert.deepEqual([...drafts.failed('host')], []); + drafts.discard(); + drafts.set('browser', 'videoMute', false); + drafts.conflict('browser', []); + assert.equal(drafts.statusText(), 'Unsaved changes.'); + drafts.conflict('browser', ['videoMute']); + assert.equal(drafts.statusText(), 'Settings changed elsewhere. Reload this section to see them.'); + assert.deepEqual([...drafts.failed('browser')], ['videoMute']); + assert.equal(drafts.get('browser', 'videoMute'), false); + assert.throws(() => drafts.reject('nowhere', 'x'), TypeError); +}); + +test('a rejected value says why in the status until the next edit', async () => { + const drafts = await loaded(adapters({ host: { result_timeout: 5 } })); + drafts.reject('host', 'result_timeout'); + assert.equal(drafts.statusText(), 'Some changes were not saved. Review the highlighted settings.'); + drafts.set('host', 'result_timeout', 9); + assert.equal(drafts.statusText(), 'Unsaved changes.'); +}); + +test('a browser write refused as changed elsewhere keeps the draft and reads the browser values again', async () => { + const domains = adapters({ browser: { workspaceDensity: 'comfortable' } }); + const drafts = await loaded(domains); + drafts.set('browser', 'workspaceDensity', 'compact'); + domains.outcome.browser = () => { throw Object.assign(new Error('changed'), { code: 'settings-changed' }); }; + assert.deepEqual(await drafts.save(), { browser: 'conflict', host: 'clean', runtime: 'clean' }); + assert.deepEqual(domains.calls.map(([name]) => name), ['writeBrowser', 'readBrowser']); + assert.equal(drafts.get('browser', 'workspaceDensity'), 'compact'); + assert.deepEqual([...drafts.failed('browser')], ['workspaceDensity']); + assert.equal(drafts.statusText(), 'Settings changed elsewhere. Reload this section to see them.'); +}); + +test('host read state says whether the server settings could be read', async () => { + const { createSystemDrafts } = await source; + const domains = adapters({ host: { result_timeout: 5 } }); + let fail = true; + const drafts = createSystemDrafts({ ...domains, readHost: async () => { if (fail) throw new Error('offline'); return { values: {}, revision: 'r' }; } }); + assert.equal(drafts.hostState(), 'pending'); + assert.equal(await drafts.reloadHost(), false); + assert.equal(drafts.hostState(), 'failed'); + fail = false; + assert.equal(await drafts.reloadHost(), true); + assert.equal(drafts.hostState(), 'loaded'); +}); diff --git a/tests/test_api_review.py b/tests/test_api_review.py index 73a9ffa..1ee2c16 100644 --- a/tests/test_api_review.py +++ b/tests/test_api_review.py @@ -68,10 +68,23 @@ class ReviewRoutesTestCase(ReviewTestCase): def _record(self, path): self._roots() - outcome = self.api.record_review_file(str(path)) + outcome = self._record_raw(path) self.assertIn(outcome, ("recorded", "limited"), outcome) return outcome + def _record_raw(self, path): + """Record ``path`` as the producers' recorder does, without their rule. + + These tests are about decisions on an item; whether the scan or the + monitor would queue the file (``tests/test_review_outcome.py``) is + not their subject, and an item can outlive the moment its file gets + linked (until the next scan or monitor pass settles it). + """ + from py.api.admin import _scan_roots + + roots = _scan_roots(self.api._get_all_output_dirs()) + return self.api._record_review_sync(Path(str(path)), roots) + async def _json(self, method, url, **kwargs): resp = await self.client.request(method, url, **kwargs) data = await resp.json() @@ -314,7 +327,7 @@ class TestQueueReads(ReviewRoutesTestCase): "Steps: 20, Sampler: Euler" ), ) - self.assertEqual(self.api.record_review_file(str(limited)), "limited") + self.assertEqual(self._record_raw(str(limited)), "limited") self._record(self._ambiguous("two.png")) items = (await self._list())["items"] self.assertEqual([i["limited"] for i in items], [True, False]) @@ -1182,7 +1195,7 @@ class TestInteractiveCapacity(ReviewRoutesTestCase): return original(store_, stream, size) def record(): - outcome["value"] = self.api.record_review_file(str(busy)) + outcome["value"] = self._record_raw(str(busy)) recorder = threading.Thread(target=record) with patch.object(CaptureStore, "_read", blocking): @@ -1193,7 +1206,7 @@ class TestInteractiveCapacity(ReviewRoutesTestCase): # Another background recording finds no worker it may use... with patch("utils.media_capture.BACKGROUND_WAIT_SECONDS", 0.01): with self.assertRaises(CaptureCapacity): - self.api.record_review_file(str(busy)) + self._record_raw(str(busy)) # ...while the interactive observe still gets the last one. observed = await self._observe(item["target"]) self.assertEqual( diff --git a/tests/test_review.py b/tests/test_review.py index e0052f3..c118cea 100644 --- a/tests/test_review.py +++ b/tests/test_review.py @@ -72,6 +72,29 @@ def ambiguous_graph(*positives): return graph +def unlinkable_graph(*positives): + """Ambiguous, and the scan's browser-parser fallback links nothing. + + Only such a file is queued (an ambiguous file the fallback links is not). + The parser reads the first sampler (node 3), whose positive input points + at a node missing from the graph; the resolver offers the other samplers + as candidates beside that unresolved one. + """ + graph = ambiguous_graph(*positives) + graph["3"] = { + "class_type": "KSampler", + "inputs": {"model": ["1", 0], "positive": ["99", 0], "negative": ["2", 0]}, + } + return graph + + +def resolved_texts(evidence): + """The positive texts of the candidates that resolved, sorted.""" + return sorted( + c["positive"]["text"] for c in evidence["candidates"] if c["positive"] + ) + + def resolved_graph(text="a red square"): return ambiguous_graph(text) @@ -155,7 +178,9 @@ class TestScanRecording(ReviewTestCase): self._roots() ambiguous = self.output_dir / "sub" / "two.png" ambiguous.parent.mkdir() - write_png(ambiguous, prompt=ambiguous_graph("cat", "dog")) + write_png(ambiguous, prompt=unlinkable_graph("cat", "dog")) + # Ambiguous too, but the browser-parser fallback links it: not queued. + write_png(self.output_dir / "linked.png", prompt=ambiguous_graph("a", "b")) write_png(self.output_dir / "resolved.png", prompt=resolved_graph()) write_png(self.output_dir / "plain.png") write_png( @@ -178,20 +203,19 @@ class TestScanRecording(ReviewTestCase): (item["state"], item["revision"], item["limited"]), ("pending", 0, 0) ) self.assertEqual(item["resolver_version"], 1) - self.assertEqual(item["candidate_count"], 2) + self.assertEqual(item["candidate_count"], 3) self.assertEqual(item["evidence_hash"], evidence_digest(item["evidence"])) evidence = json.loads(item["evidence"]) self.assertEqual(canonical_evidence(evidence), item["evidence"]) self.assertEqual(evidence["resolver_version"], 1) - self.assertEqual( - sorted(c["positive"]["text"] for c in evidence["candidates"]), - ["cat", "dog"], - ) + self.assertEqual(resolved_texts(evidence), ["cat", "dog"]) for candidate in evidence["candidates"]: self.assertEqual( set(candidate), {"hash", "sampler_id", "state", "reason", "positive", "negative"}, ) + if candidate["positive"] is None: # the parser's unresolved sampler + continue self.assertEqual( set(candidate["positive"]), {"text", "storage_text", "provenance"} ) @@ -210,7 +234,7 @@ class TestScanRecording(ReviewTestCase): async def test_scan_statuses_counters_and_outcome_rows_unchanged(self): self._require_module() self._roots() - write_png(self.output_dir / "two.png", prompt=ambiguous_graph("cat", "dog")) + write_png(self.output_dir / "two.png", prompt=unlinkable_graph("cat", "dog")) write_png(self.output_dir / "one.png", prompt=resolved_graph()) write_png( self.output_dir / "delims.png", @@ -243,8 +267,8 @@ class TestScanRecording(ReviewTestCase): with self.assertLogs("prompt_manager.api", logging.WARNING) as logs: without = run(other, broken) self.assertEqual(recorded, without) - self.assertEqual(recorded[0]["ambiguous"], 1) - self.assertEqual(recorded[0]["added"], 2) + self.assertEqual(recorded[0]["ambiguous"], 2) + self.assertEqual(recorded[0]["added"], 1) self.assertTrue(any("Needs review" in line for line in logs.output)) self.assertEqual(len(self._items()), 2) other.close_annotation_captures() @@ -258,7 +282,7 @@ class TestScanRecording(ReviewTestCase): async def test_new_scope_rereads_ambiguous_files_and_stays_idempotent(self): self._require_module() self._roots() - write_png(self.output_dir / "two.png", prompt=ambiguous_graph("cat", "dog")) + write_png(self.output_dir / "two.png", prompt=unlinkable_graph("cat", "dog")) write_png(self.output_dir / "one.png", prompt=resolved_graph()) first = await self._scan("new") revision = self._review_revision() @@ -274,8 +298,11 @@ class TestScanRecording(ReviewTestCase): self.assertEqual(reads, ["two.png"]) self.assertEqual(first["scope"]["review_items"], 1) - self.assertEqual(second["scope"]["review_items"], 0) + # Still in the queue (review_items), but nothing new was added. + self.assertEqual(second["scope"]["review_items"], 1) + self.assertEqual(second["scope"]["review_added"], 0) self.assertNotIn("added to Needs review", "\n".join(second["log"])) + self.assertIn("? 1 image already in Needs review", "\n".join(second["log"])) self.assertEqual(len(self._items()), 1) self.assertEqual(self._review_revision(), revision) self.assertEqual(self._capture_files(), []) @@ -287,7 +314,7 @@ class TestScanRecording(ReviewTestCase): self._require_module() self._roots() path = self.output_dir / "two.png" - write_png(path, prompt=ambiguous_graph("cat", "dog")) + write_png(path, prompt=unlinkable_graph("cat", "dog")) await self._scan("new") captures = [] original = CaptureStore.capture @@ -308,7 +335,8 @@ class TestScanRecording(ReviewTestCase): second = await self._scan("new") self.assertEqual(captures, []) self.assertEqual(reads, ["two.png"]) - self.assertEqual(second["scope"]["review_items"], 0) + self.assertEqual(second["scope"]["review_added"], 0) + self.assertEqual(second["scope"]["review_items"], 1) # A touched file (same bytes, new mtime) is captured again: a # duplicate of its item, whose fingerprint is refreshed. stat = path.stat() @@ -323,7 +351,7 @@ class TestScanRecording(ReviewTestCase): async def test_repeat_scan_keeps_decisions(self): self._require_module() self._roots() - write_png(self.output_dir / "two.png", prompt=ambiguous_graph("cat", "dog")) + write_png(self.output_dir / "two.png", prompt=unlinkable_graph("cat", "dog")) await self._scan("all") item = self._items()[0] candidate = json.loads(item["evidence"])["candidates"][0]["hash"] @@ -362,10 +390,10 @@ class TestScanRecording(ReviewTestCase): self._require_module() self._roots() path = self.output_dir / "two.png" - write_png(path, prompt=ambiguous_graph("cat", "dog")) + write_png(path, prompt=unlinkable_graph("cat", "dog")) await self._scan() old = self._items()[0] - write_png(path, prompt=ambiguous_graph("cat", "dog"), color=(0, 0, 255)) + write_png(path, prompt=unlinkable_graph("cat", "dog"), color=(0, 0, 255)) self.assertNotEqual(digest(path), old["sha256"]) job = await self._scan() @@ -387,7 +415,7 @@ class TestScanRecording(ReviewTestCase): parameters=DELIMITERS, ) many = [f"prompt {n}" for n in range(17)] - write_png(self.output_dir / "many.png", prompt=ambiguous_graph(*many)) + write_png(self.output_dir / "many.png", prompt=unlinkable_graph(*many)) job = await self._scan() @@ -399,14 +427,16 @@ class TestScanRecording(ReviewTestCase): self.assertEqual(evidence["candidates"], []) self.assertIs(evidence["limited"], True) self.assertEqual( - json.loads(items["many.png"]["evidence"])["candidate_total"], 17 + # 17 samplers and the parser's unresolved one. + json.loads(items["many.png"]["evidence"])["candidate_total"], + 18, ) self.assertEqual(job["scope"]["review_items"], 2) async def test_capacity_errors_are_logged_and_counted_not_fatal(self): self._require_module() self._roots() - write_png(self.output_dir / "two.png", prompt=ambiguous_graph("cat", "dog")) + write_png(self.output_dir / "two.png", prompt=unlinkable_graph("cat", "dog")) def full(*args, **kwargs): raise CaptureCapacity("Too many captures are in progress") @@ -415,8 +445,8 @@ class TestScanRecording(ReviewTestCase): job = await self._scan() self.assertEqual(job["state"], "finished") - # The browser parser links this ambiguous file: Activity counts it Added. - self.assertEqual(job["counters"]["added"], 1) + # Nothing links this ambiguous file: Activity counts it under review. + self.assertEqual(job["counters"]["review"], 1) self.assertEqual(job["scope"]["review_items"], 0) self.assertEqual(job["scope"]["review_errors"], 1) self.assertIn( @@ -427,7 +457,7 @@ class TestScanRecording(ReviewTestCase): async def test_legacy_scan_events_are_unchanged(self): self._require_module() self._roots() - write_png(self.output_dir / "two.png", prompt=ambiguous_graph("cat", "dog")) + write_png(self.output_dir / "two.png", prompt=unlinkable_graph("cat", "dog")) resp = await self.client.request("POST", "/prompt_manager/scan") payloads = _sse_payloads(await resp.text()) final = payloads[-1] @@ -755,15 +785,17 @@ class TestMonitorRecording(ReviewTestCase): ).fetchone() return row[0] if row else None - async def test_monitor_keeps_fallback_link_and_records_item_best_effort(self): + async def test_monitor_keeps_its_link_and_never_queues_the_linked_file(self): + """The monitor's pass is best effort (a failure is logged, the link + stays); a file the monitor linked is never queued.""" self._require_module() self._roots() path = self.output_dir / "two.png" - write_png(path, prompt=ambiguous_graph("cat", "dog")) + write_png(path, prompt=unlinkable_graph("cat", "dog")) prompt_id = self.api.db.save_prompt( text="latest", prompt_hash=generate_prompt_hash("latest") ) - metadata = {"prompt": ambiguous_graph("cat", "dog")} + metadata = {"prompt": unlinkable_graph("cat", "dog")} context = {"id": prompt_id, "text": "latest", "fallback": True} handler = self._handler() @@ -779,13 +811,18 @@ class TestMonitorRecording(ReviewTestCase): self.assertEqual(self._items(), []) other = self.output_dir / "three.png" - write_png(other, prompt=ambiguous_graph("cat", "dog")) - handler.review_recorder = self.api.record_review_file + write_png(other, prompt=unlinkable_graph("cat", "dog")) + calls = [] + + def recorder(image_path): + calls.append(self.api.record_review_file(image_path)) + return calls[-1] + + handler.review_recorder = recorder handler.link_image_to_prompt(str(other), context, metadata) self.assertEqual(self._linked_prompt(other), prompt_id) - items = self._items() - self.assertEqual(len(items), 1) - self.assertEqual(items[0]["sha256"], digest(other)) + self.assertEqual(calls, ["linked"]) + self.assertEqual(self._items(), []) self.assertEqual(self._capture_files(), []) async def test_monitor_skips_resolved_and_files_outside_the_roots(self): @@ -822,7 +859,7 @@ class TestMonitorRecording(ReviewTestCase): self._roots() path = self.output_dir / "held.png" - write_png(path, prompt=ambiguous_graph("cat", "dog")) + write_png(path, prompt=unlinkable_graph("cat", "dog")) for kind, admission in ( (RESTORE_JOB_KIND, "records"), @@ -912,7 +949,7 @@ class TestRecordedSpelling(ReviewTestCase): self._require_module() self._roots() upper = self.output_dir / "Two.PNG" - write_png(upper, prompt=ambiguous_graph("cat", "dog")) + write_png(upper, prompt=unlinkable_graph("cat", "dog")) with _case_folding(): self.assertEqual(self.api.record_review_file(str(upper)), "recorded") (before,) = self._items() @@ -992,7 +1029,7 @@ class TestCaptureStoreShutdown(ReviewTestCase): self._require_module() self._roots() path = self.output_dir / "two.png" - write_png(path, prompt=ambiguous_graph("cat", "dog")) + write_png(path, prompt=unlinkable_graph("cat", "dog")) await self.api.cleanup_annotation_captures(None) # the shutdown hook self.assertIsNone(self.api._captures) with patch("utils.media_capture.tempfile.mkdtemp") as mkdtemp: diff --git a/tests/test_review_outcome.py b/tests/test_review_outcome.py new file mode 100644 index 0000000..3c36658 --- /dev/null +++ b/tests/test_review_outcome.py @@ -0,0 +1,372 @@ +"""Needs review takes only files whose final outcome is ambiguous and unlinked. + +The maintainer's live scan (#284 / #288) queued every file the strict +resolver found ambiguous, also those the legacy browser-parser fallback then +linked and those already linked, so the queue (10,232) was far larger than +the scan's Needs review group. The rule now: a file is queued only when the +resolver finds it ambiguous, the fallback does not link it and no saved row +links it. A pending, undecided item whose file is linked (by the fallback, a +later resolution or the monitor) is removed on the next scan or monitor +pass; an item that carries a decision is never touched. +""" + +import json +import logging +import os +import sqlite3 +import unittest +from unittest.mock import patch + +try: + from tests.test_review import ( + DELIMITERS, + NOW, + ReviewTestCase, + ambiguous_graph, + resolved_graph, + unlinkable_graph, + write_png, + ) +except ImportError: # discovered with tests/ as the top-level directory + from test_review import ( + DELIMITERS, + NOW, + ReviewTestCase, + ambiguous_graph, + resolved_graph, + unlinkable_graph, + write_png, + ) + +from utils.hashing import generate_prompt_hash +from utils.image_monitor import ImageGenerationHandler +from utils.prompt_tracker import PromptTracker +from utils import review_capture + + +def nan_stamped(graph): + """ComfyUI's ``is_changed: [NaN]`` stamp (written as a bare NaN literal).""" + for node in graph.values(): + node["is_changed"] = [float("nan")] + return graph + + +class OutcomeTestCase(ReviewTestCase): + def _pending(self): + return self.api.db.library_counts()["review_pending"] + + def _record(self, name): + """An item as the pre-fix producers recorded it (any ambiguous file).""" + outcome = review_capture.record_ambiguous_file( + self.api.db, self.api._captures, self.output_dir, 0, name + ) + self.assertIn(outcome, ("recorded", "limited")) + + def _decide(self, item, outcome): + conn = self.api.db.model.get_connection() + candidate = None + if outcome == "selected": + candidate = json.loads(item["evidence"])["candidates"][0]["hash"] + with conn: + conn.execute( + "UPDATE media_review_items SET state=?, revision=1 WHERE id=?", + (outcome, item["id"]), + ) + conn.execute( + "INSERT INTO media_review_decisions(id,item_id,decision_revision," + "created_at,outcome,candidate_hash,evidence) VALUES(?,?,?,?,?,?,?)", + ( + f"00000000-0000-4000-8000-{len(self._decisions()) + 1:012d}", + item["id"], + 1, + NOW, + outcome, + candidate, + '{"kind":"user-selection"}', + ), + ) + + def _decisions(self): + conn = self.api.db.model.get_connection() + return [ + tuple(row) + for row in conn.execute( + "SELECT id,item_id,outcome FROM media_review_decisions ORDER BY id" + ) + ] + + def _link(self, path, text="already saved"): + prompt_id = self.api.db.save_prompt( + text=text, prompt_hash=generate_prompt_hash(text) + ) + self.api.db.link_image_to_prompt(prompt_id, str(path)) + return prompt_id + + @staticmethod + def _rows(job, group): + return sorted(row["path"] for row in job["outcomes"].get(group, [])) + + +class TestOnlyUnlinkedAmbiguousFilesAreQueued(OutcomeTestCase): + async def test_nan_stamped_graph_the_fallback_links_is_not_queued(self): + self._roots() + path = self.output_dir / "nan.png" + write_png(path, prompt=nan_stamped(ambiguous_graph("cat", "dog"))) + self.assertIn(b"NaN", path.read_bytes()) + + job = await self._scan() + + self.assertEqual(job["counters"]["added"], 1, job["counters"]) + self.assertEqual(self._items(), []) + self.assertEqual(job["scope"]["review_items"], 0) + self.assertEqual(self._pending(), 0) + + async def test_fallback_link_to_a_known_prompt_is_not_queued(self): + self._roots() + write_png(self.output_dir / "a.png", prompt=ambiguous_graph("cat", "dog")) + await self._scan() + write_png(self.output_dir / "b.png", prompt=ambiguous_graph("cat", "dog")) + + job = await self._scan() + + self.assertEqual(self._items(), []) + self.assertEqual(job["scope"]["review_items"], 0) + + async def test_already_linked_file_is_not_queued(self): + self._roots() + path = self.output_dir / "linked.png" + write_png(path, prompt=unlinkable_graph("cat", "dog")) + self._link(path) + + job = await self._scan() + + self.assertEqual(self._items(), []) + self.assertEqual(job["scope"]["review_items"], 0) + self.assertEqual(self._rows(job, "review"), []) + self.assertEqual(self._rows(job, "known"), ["linked.png"]) + + async def test_genuinely_ambiguous_file_is_queued(self): + self._roots() + path = self.output_dir / "two.png" + write_png(path, prompt=nan_stamped(unlinkable_graph("cat", "dog"))) + # Beside it, a file the fallback links: only the first is queued. + write_png(self.output_dir / "linked.png", prompt=ambiguous_graph("a", "b")) + + job = await self._scan() + + (item,) = self._items() + self.assertEqual(os.path.basename(item["locator"]), "two.png") + self.assertEqual(item["state"], "pending") + self.assertEqual(item["candidate_count"], 3) + self.assertEqual(job["scope"]["review_items"], 1) + self.assertEqual(self._rows(job, "review"), ["two.png"]) + self.assertEqual(self._pending(), 1) + self.assertIn("? 1 image added to Needs review", "\n".join(job["log"])) + + async def test_counter_and_queue_agree_with_the_review_group(self): + """``review_items`` and the queue total equal the review group less + its unresolved files, on the first scan and on a rescan.""" + self._roots() + for name in ("u1.png", "u2.png"): + write_png(self.output_dir / name, prompt=unlinkable_graph("cat", name)) + write_png(self.output_dir / "delims.png", parameters=DELIMITERS) + write_png(self.output_dir / "linked.png", prompt=ambiguous_graph("a", "b")) + unresolved = {"1": {"class_type": "KSampler", "inputs": {}}} + write_png(self.output_dir / "none.png", prompt=unresolved) + write_png(self.output_dir / "one.png", prompt=resolved_graph()) + + for _ in range(2): + job = await self._scan() + review = self._rows(job, "review") + unresolved_rows = sorted( + row["path"] + for row in job["outcomes"]["review"] + if "could not be traced" in row["detail"] + ) + queued = [name for name in review if name not in unresolved_rows] + self.assertEqual(queued, ["delims.png", "u1.png", "u2.png"], job) + self.assertEqual(job["scope"]["review_items"], 3) + self.assertEqual(self._pending(), 3) + self.assertIn("? 3 images already in Needs review", "\n".join(job["log"])) + + +class TestQueueSelfHeals(OutcomeTestCase): + async def test_pending_item_of_a_fallback_linked_file_is_removed(self): + """The maintainer's case: queued by the old rule, linked on rescan.""" + self._roots() + path = self.output_dir / "nan.png" + write_png(path, prompt=nan_stamped(ambiguous_graph("cat", "dog"))) + self._record("nan.png") + revision = self._review_revision() + self.assertEqual(self._pending(), 1) + + job = await self._scan() + + self.assertEqual(self._items(), []) + self.assertEqual(self._pending(), 0) + self.assertGreater(self._review_revision(), revision) + self.assertEqual(job["scope"]["review_settled"], 1) + self.assertIn("= 1 image left Needs review · now linked", "\n".join(job["log"])) + + async def test_pending_item_of_an_already_linked_file_is_removed(self): + self._roots() + path = self.output_dir / "linked.png" + write_png(path, prompt=unlinkable_graph("cat", "dog")) + self._record("linked.png") + self._link(path) + + await self._scan() + + self.assertEqual(self._items(), []) + self.assertEqual(self._pending(), 0) + + async def test_pending_item_of_a_skipped_known_file_is_removed(self): + """A file the "new" scope skips as known still settles its old item.""" + self._roots() + path = self.output_dir / "one.png" + write_png(path, prompt=unlinkable_graph("cat", "dog")) + self._record("one.png") + write_png(path, prompt=resolved_graph()) + first = await self._scan() + self.assertEqual(self._rows(first, "added"), ["one.png"]) + # An item a pre-fix scan left for the file's old bytes. + self.assertEqual(self._items(), []) + write_png(path, prompt=unlinkable_graph("cat", "dog"), color=(0, 9, 0)) + self._record("one.png") + write_png(path, prompt=resolved_graph()) + second = await self._scan() + self.assertEqual(self._rows(second, "known"), ["one.png"]) + self.assertEqual(self._items(), []) + + async def test_a_failed_settle_is_logged_and_counted_not_fatal(self): + self._roots() + write_png(self.output_dir / "nan.png", prompt=ambiguous_graph("cat", "dog")) + self._record("nan.png") + + with ( + patch.object( + self.api.db, + "settle_linked_review_items", + side_effect=sqlite3.OperationalError("database is locked"), + ), + self.assertLogs("prompt_manager.api", logging.WARNING) as logs, + ): + job = await self._scan() + + self.assertEqual(job["state"], "finished") + self.assertEqual(job["scope"]["review_errors"], 1) + self.assertEqual(job["scope"]["review_settled"], 0) + self.assertTrue(any("could not be removed" in line for line in logs.output)) + self.assertEqual(self._pending(), 1) + # The next scan settles it. + await self._scan() + self.assertEqual(self._pending(), 0) + + async def test_still_unlinked_pending_item_stays(self): + self._roots() + write_png(self.output_dir / "two.png", prompt=unlinkable_graph("cat", "dog")) + await self._scan() + (item,) = self._items() + + job = await self._scan() + + self.assertEqual([i["id"] for i in self._items()], [item["id"]]) + self.assertEqual(job["scope"].get("review_settled", 0), 0) + + async def test_decided_items_are_never_touched(self): + self._roots() + for name in ("sel.png", "unres.png"): + write_png(self.output_dir / name, prompt=ambiguous_graph("cat", name)) + self._record(name) + items = {os.path.basename(i["locator"]): i for i in self._items()} + self._decide(items["sel.png"], "selected") + self._decide(items["unres.png"], "unresolved") + before = (self._items(), self._decisions()) + # A pending neighbour the fallback links: it alone leaves the queue. + write_png(self.output_dir / "pending.png", prompt=ambiguous_graph("x", "y")) + self._record("pending.png") + self.assertEqual(self._pending(), 1) + + job = await self._scan("all") + + self.assertEqual((self._items(), self._decisions()), before) + self.assertEqual(self._pending(), 0) + self.assertEqual(job["scope"]["review_settled"], 1) + + +class TestMonitorRule(OutcomeTestCase): + def _handler(self): + handler = ImageGenerationHandler(self.api.db, PromptTracker(self.api.db)) + handler.review_recorder = self.api.record_review_file + return handler + + def _context(self): + prompt_id = self.api.db.save_prompt( + text="latest", prompt_hash=generate_prompt_hash("latest") + ) + return {"id": prompt_id, "text": "latest", "fallback": True} + + async def test_monitor_linked_ambiguous_output_is_not_queued(self): + self._roots() + path = self.output_dir / "two.png" + graph = unlinkable_graph("cat", "dog") + write_png(path, prompt=graph) + + self._handler().link_image_to_prompt( + str(path), self._context(), {"prompt": graph} + ) + + self.assertEqual(self._items(), []) + + async def test_monitor_link_removes_the_files_pending_item(self): + self._roots() + path = self.output_dir / "two.png" + graph = unlinkable_graph("cat", "dog") + write_png(path, prompt=graph) + self._record("two.png") + self.assertEqual(self._pending(), 1) + + self._handler().link_image_to_prompt( + str(path), self._context(), {"prompt": graph} + ) + + self.assertEqual(self._items(), []) + self.assertEqual(self._pending(), 0) + + async def test_monitor_never_touches_a_decided_item(self): + self._roots() + path = self.output_dir / "two.png" + graph = unlinkable_graph("cat", "dog") + write_png(path, prompt=graph) + self._record("two.png") + (item,) = self._items() + self._decide(item, "unresolved") + before = (self._items(), self._decisions()) + other = self.output_dir / "other.png" + write_png(other, prompt=graph, color=(0, 0, 9)) + self._record("other.png") + handler, context = self._handler(), self._context() + + handler.link_image_to_prompt(str(path), context, {"prompt": graph}) + handler.link_image_to_prompt(str(other), context, {"prompt": graph}) + + self.assertEqual((self._items(), self._decisions()), before) + self.assertEqual(self._pending(), 0) + + async def test_record_review_file_applies_the_scan_rule(self): + self._roots() + write_png(self.output_dir / "fallback.png", prompt=ambiguous_graph("a", "b")) + write_png(self.output_dir / "two.png", prompt=unlinkable_graph("cat", "dog")) + linked = self.output_dir / "linked.png" + write_png(linked, prompt=unlinkable_graph("cat", "dog")) + self._link(linked) + + record = self.api.record_review_file + self.assertEqual(record(str(self.output_dir / "fallback.png")), "skipped") + self.assertEqual(record(str(linked)), "linked") + self.assertEqual(record(str(self.output_dir / "two.png")), "recorded") + self.assertEqual(len(self._items()), 1) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_system_api.py b/tests/test_system_api.py index 3d649fe..99891bd 100644 --- a/tests/test_system_api.py +++ b/tests/test_system_api.py @@ -416,51 +416,60 @@ class TestStatisticsRoute(SystemAPITestCase): ) -class TestReadsSettleBeforeTheAnswer(SystemAPITestCase): - """A failed read answers only after its sibling reads ended. +class TestFailedReadsSettleFirst(SystemAPITestCase): + """A failed read answers only after the route's other reads finished. - ``asyncio.gather`` does not stop executor work, so an early answer left a - sibling read on the database while the caller went on (in the suite, the - test's teardown deleted the database under it: a segfault on py3.10). + Answering on the first failure (``asyncio.gather`` without + ``return_exceptions``) left the other executor reads running against a + database the caller may already be closing; on Python 3.10 CI that + crashed the test process inside SQLite. """ - def _slow(self, target, name): - real = getattr(target, name) - finished = threading.Event() + async def assert_answers_after_reads(self, url, failing, slow, error): + raised, release, finished = (threading.Event() for _ in range(3)) + real = getattr(*slow) - def slow(*args, **kwargs): - threading.Event().wait(0.3) # still reading when the sibling fails + def fail(*args, **kwargs): + raised.set() + raise RuntimeError("secret") + + def held(*args, **kwargs): + release.wait() try: return real(*args, **kwargs) finally: finished.set() - return patch.object(target, name, side_effect=slow), finished - - async def _failing(self, url, failing, slow_target, slow_name): - slow, finished = self._slow(slow_target, slow_name) with ( - slow, - patch.object(self.api.db, failing, side_effect=RuntimeError("secret")), + patch.object(*failing, side_effect=fail), + patch.object(*slow, side_effect=held), self.assertLogs("prompt_manager.api", "ERROR"), ): - await self.json("GET", url, 500) - self.assertTrue(finished.is_set(), "a sibling read outlived the answer") + answer = asyncio.ensure_future(self.json("GET", url, 500)) + try: + await asyncio.get_running_loop().run_in_executor(None, raised.wait) + done, _ = await asyncio.wait({answer}, timeout=0.5) + self.assertEqual(done, set(), "answered while a read still ran") + finally: + release.set() + body = await answer + self.assertTrue(finished.is_set()) + self.assertEqual(body, {"success": False, "code": None, "error": error}) - async def test_statistics_failure_waits_for_its_sibling_reads(self): - await self._failing( + async def test_statistics(self): + await self.assert_answers_after_reads( "/prompt_manager/system/statistics", - "usage_by_category", - self.api.db, - "get_statistics", + (self.api.db, "usage_by_category"), + (self.api.db, "get_statistics"), + "Failed to read statistics", ) - async def test_overview_failure_waits_for_its_sibling_reads(self): - await self._failing( + async def test_overview(self): + await self.assert_answers_after_reads( "/prompt_manager/system/overview", - "library_counts", - self.api, - "_output_dirs_with_origin", + (self.api.db, "library_counts"), + (self.api, "_output_dirs_with_origin"), + "Failed to read the overview", ) diff --git a/tests/test_workspace_system_fixture.py b/tests/test_workspace_system_fixture.py new file mode 100644 index 0000000..a43ec85 --- /dev/null +++ b/tests/test_workspace_system_fixture.py @@ -0,0 +1,38 @@ +"""The System browser hooks only ever reset their own server's config.json (#249 S3).""" + +import os +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +from tests.browser.workspace_system_fixture import register_system_commands + + +class TestWorkspaceSystemFixture(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + self.root = Path(self.tmp.name) / "server" + self.root.mkdir() + + def test_refuses_a_config_path_outside_its_root(self): + elsewhere = Path(self.tmp.name) / "config.json" + elsewhere.write_bytes(b'{"real": true}') + for value in (str(elsewhere), None): + env = { + k: v for k, v in os.environ.items() if k != "PROMPT_MANAGER_CONFIG_PATH" + } + if value is not None: + env["PROMPT_MANAGER_CONFIG_PATH"] = value + with ( + self.subTest(value=value), + mock.patch.dict(os.environ, env, clear=True), + ): + with self.assertRaisesRegex(RuntimeError, "PROMPT_MANAGER_CONFIG_PATH"): + register_system_commands(None, None, None, self.root) + self.assertEqual(elsewhere.read_bytes(), b'{"real": true}') + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/vendor/frontend-coverage-baseline.json b/tools/vendor/frontend-coverage-baseline.json index 9c14652..b03d7de 100644 --- a/tools/vendor/frontend-coverage-baseline.json +++ b/tools/vendor/frontend-coverage-baseline.json @@ -37,15 +37,15 @@ }, "gallery-settings.js": { "lines": 98, - "branches": 90, + "branches": 94, "functions": 98, "statements": 98 }, "gallery.js": { - "lines": 40, + "lines": 41, "branches": 63, "functions": 44, - "statements": 40 + "statements": 41 }, "html-escape.js": { "lines": 98, @@ -407,6 +407,12 @@ "functions": 98, "statements": 98 }, + "workspace/preferences.mjs": { + "lines": 98, + "branches": 96, + "functions": 98, + "statements": 98 + }, "workspace/prompt-blocks.mjs": { "lines": 90, "branches": 92, @@ -461,6 +467,12 @@ "functions": 98, "statements": 98 }, + "workspace/system-appearance.mjs": { + "lines": 98, + "branches": 93, + "functions": 98, + "statements": 98 + }, "workspace/system-controls.mjs": { "lines": 98, "branches": 87, @@ -475,15 +487,15 @@ }, "workspace/system-state.mjs": { "lines": 98, - "branches": 92, - "functions": 92, + "branches": 94, + "functions": 98, "statements": 98 }, "workspace/system.mjs": { - "lines": 93, - "branches": 85, - "functions": 69, - "statements": 93 + "lines": 98, + "branches": 92, + "functions": 85, + "statements": 98 }, "workspace/theme.mjs": { "lines": 98, diff --git a/utils/image_monitor.py b/utils/image_monitor.py index 0af5016..cfe1a6a 100644 --- a/utils/image_monitor.py +++ b/utils/image_monitor.py @@ -75,8 +75,9 @@ class ImageGenerationHandler(FileSystemEventHandler): self.prompt_tracker = prompt_tracker self.metadata_extractor = ComfyUIMetadataExtractor() self.logger = get_logger("prompt_manager.image_monitor") - # Needs review recording for ambiguous outputs (the API's - # ``record_review_file``, wired at start-up); None records nothing. + # The Needs review pass for ambiguous outputs (the API's + # ``record_review_file``, wired at start-up): a linked output is + # never queued and its pending items leave the queue. None: no pass. self.review_recorder: Optional[Callable[[str], str]] = None # Read from GalleryConfig if available, otherwise use defaults @@ -587,10 +588,11 @@ class ImageGenerationHandler(FileSystemEventHandler): self._record_review(image_path) def _record_review(self, image_path: str) -> None: - """Queue an ambiguous output for Needs review, best effort. + """The Needs review pass for an ambiguous output, best effort. - The fallback link above stays whatever happens here: a failure is - logged, never raised. + The output is linked by now, so it is never queued; a pending item + an earlier scan left for its path leaves the queue. The link above + stays whatever happens here: a failure is logged, never raised. """ recorder = getattr(self, "review_recorder", None) if recorder is None: diff --git a/utils/review_capture.py b/utils/review_capture.py index 90d0ee0..31b9e2b 100644 --- a/utils/review_capture.py +++ b/utils/review_capture.py @@ -53,15 +53,17 @@ def record_ambiguous_file(db, store, root, root_index: int, relative: str) -> st Returns ``recorded`` (new item with candidates), ``limited`` (new item without candidates), ``duplicate`` (these bytes already carry this evidence, or the file's current stat is the verified fingerprint of - bytes an item cites, so it is not even copied) or ``skipped`` (the file + bytes an item cites, so it is not even copied, and that item is still + pending), ``decided`` (as ``duplicate``, but the item carries a decision, + so the file is not in the queue) or ``skipped`` (the file changed, vanished, is not ambiguous in its captured bytes, or storage was restored meanwhile). ``CaptureCapacity`` and database errors propagate to the caller, which logs and counts them. """ - spelled_path = _queued_spelling(db, root, relative) - if spelled_path is None: - return "duplicate" + spelled_path, queued = _queued_spelling(db, root, relative) + if queued is not None: + return _already_queued(db, *queued) epoch = db.get_execution_epoch() try: # Background: the last worker stays free for an interactive request. @@ -102,12 +104,22 @@ def record_ambiguous_file(db, store, root, root_index: int, relative: str) -> st except LibraryCommandError: return "skipped" if not inserted: - return "duplicate" + return _already_queued(db, capture.locator, capture.size, capture.mtime_ns) return "limited" if limited else "recorded" -def _queued_spelling(db, root, relative: str) -> str | None: - """The file's spelling, or None when its current stat is already queued. +def _already_queued(db, locator: str, size: int, mtime_ns: int) -> str: + """``duplicate`` while the existing item is pending, else ``decided``.""" + if db.review_pending_at(locator, size, mtime_ns, RESOLVER_VERSION): + return "duplicate" + return "decided" + + +def _queued_spelling(db, root, relative: str) -> tuple[str, tuple | None]: + """The file's spelling, and ``(locator, size, mtime_ns)`` when queued. + + The second value is None unless the file's current stat is already + queued. The spelling is the capture's own candidate path (the real root joined with the listed relative path), so it folds to the capture's locator. A @@ -121,7 +133,7 @@ def _queued_spelling(db, root, relative: str) -> str | None: try: facts = os.lstat(spelled) except OSError: - return spelled + return spelled, None locator = os.path.normcase(spelled) if db.review_fingerprint_queued( locator, facts.st_size, facts.st_mtime_ns, RESOLVER_VERSION @@ -129,8 +141,8 @@ def _queued_spelling(db, root, relative: str) -> str | None: # Not copied again, but a case-only rename keeps the stat: record # the current spelling so the item's target still opens. db.refresh_review_spelling(locator, spelled) - return None - return spelled + return spelled, (locator, facts.st_size, facts.st_mtime_ns) + return spelled, None def captured_review(store, capture) -> tuple[dict, dict] | None: diff --git a/web/js/gallery-settings.js b/web/js/gallery-settings.js index 4c21dd6..3ea591d 100644 --- a/web/js/gallery-settings.js +++ b/web/js/gallery-settings.js @@ -4,6 +4,9 @@ * * Settings are per browser (localStorage key "gallerySettings"). Anything * stored can be stale or hand-edited, so every value is checked before use. + * The workspace shares this object ("One display configuration", #249 System + * S3): it adds workspaceLayout and workspaceDensity, and every key either page + * does not know is kept. * * Loadable in the browser (window.GallerySettings) and in Node for tests. */ @@ -11,6 +14,9 @@ "use strict"; const VIEW_MODES = ["grid", "list"]; + // The workspace Images defaults; the legacy gallery's defaultViewMode has no masonry. + const WORKSPACE_LAYOUTS = ["grid", "list", "masonry"]; + const WORKSPACE_DENSITIES = ["comfortable", "large", "compact"]; const LIMIT_OPTIONS = [50, 100, 200, 500]; const MIN_COLUMNS = 2; const MAX_COLUMNS = 12; @@ -34,6 +40,8 @@ videoAutoplay: false, videoMute: true, videoLoop: true, + workspaceLayout: "grid", + workspaceDensity: "comfortable", }); function toInt(value) { @@ -72,6 +80,8 @@ videoAutoplay: bool(merged.videoAutoplay, DEFAULT_SETTINGS.videoAutoplay), videoMute: bool(merged.videoMute, DEFAULT_SETTINGS.videoMute), videoLoop: bool(merged.videoLoop, DEFAULT_SETTINGS.videoLoop), + workspaceLayout: WORKSPACE_LAYOUTS.includes(merged.workspaceLayout) ? merged.workspaceLayout : DEFAULT_SETTINGS.workspaceLayout, + workspaceDensity: WORKSPACE_DENSITIES.includes(merged.workspaceDensity) ? merged.workspaceDensity : DEFAULT_SETTINGS.workspaceDensity, }; } @@ -81,7 +91,7 @@ return `repeat(${n}, minmax(0, 1fr))`; } - const api = { DEFAULT_SETTINGS, LIMIT_OPTIONS, VIEW_MODES, normalizeGallerySettings, gridColumnsStyle }; + const api = { DEFAULT_SETTINGS, LIMIT_OPTIONS, VIEW_MODES, WORKSPACE_LAYOUTS, WORKSPACE_DENSITIES, normalizeGallerySettings, gridColumnsStyle }; if (typeof module !== "undefined" && module.exports) module.exports = api; else root.GallerySettings = api; })(typeof window !== "undefined" ? window : globalThis); diff --git a/web/js/gallery.js b/web/js/gallery.js index ee9f24b..cad3c2c 100644 --- a/web/js/gallery.js +++ b/web/js/gallery.js @@ -1768,7 +1768,10 @@ Seed: ${this.currentMetadata.seed || 'Unknown'}`; } saveSettings() { + // The workspace shares this object (its layout and density, #249 System S3), so a Save starts from what is + // stored and keeps every key this dialog does not edit. const settings = { + ...this.getSettings(), lazyLoading: document.getElementById('lazyLoadingToggle').checked, imageQuality: document.getElementById('imageQualitySelect').value, defaultViewMode: document.getElementById('defaultViewModeSelect').value, @@ -1782,8 +1785,6 @@ Seed: ${this.currentMetadata.seed || 'Unknown'}`; infiniteScroll: document.getElementById('infiniteScrollToggle').checked, debugMode: document.getElementById('debugModeToggle').checked, apiTimeout: parseInt(document.getElementById('apiTimeoutInput').value), - thumbnailsGenerated: this.getSettings().thumbnailsGenerated, // Preserve this - thumbnailPromptDismissed: this.getSettings().thumbnailPromptDismissed, checkThumbnailsAtStartup: document.getElementById('checkThumbnailsAtStartup').checked, // Video settings videoAutoplay: document.getElementById('videoAutoplayToggle').checked, diff --git a/web/js/workspace/file-inspector.mjs b/web/js/workspace/file-inspector.mjs index 2d7a032..80465ac 100644 --- a/web/js/workspace/file-inspector.mjs +++ b/web/js/workspace/file-inspector.mjs @@ -6,6 +6,7 @@ import { createWorkspaceIcon } from './icons.mjs'; import { navigateTo } from './state.mjs'; import { showWorkspaceToast } from './toast.mjs'; import { openCopyFallback } from './workspace-dialogs.mjs'; +import { displayPreferences } from './preferences.mjs'; const RAW_KEYS = ['prompt', 'workflow', 'parameters', 'comfyassets_prompt_pairs']; const RAW_LIMIT = 'Raw metadata exceeds the response limit. Download the original file to inspect it externally.'; @@ -196,7 +197,10 @@ function prefixedChip(prefix, text) { * `variant: 'full'` (the viewer's `#viewer-details`, imageDetails(i, true)) leaves out the cover and adds "File & * provenance" and "Workflow & raw metadata" with the downloads. `inline: true` means `panel`; neither means `full`. */ -export function mountFileInspector(container, {api, sender, parentScope, variant, inline = false, promptContext = '', onTagFilter, onRetry, onRefresh, onInteraction = () => {}, copyFallback = openCopyFallback}) { +/** D124: the inspector's line while "Load metadata on selection" is off. */ +export const DEFERRED_TEXT = 'Metadata loads when you ask for it.'; + +export function mountFileInspector(container, {api, sender, parentScope, variant, inline = false, promptContext = '', onTagFilter, onRetry, onRefresh, onLoad, onInteraction = () => {}, copyFallback = openCopyFallback}) { if (parentScope.signal.aborted) return {setState() {}, dispose() {}}; const scope = childScope(parentScope); let contentScope; @@ -235,6 +239,13 @@ export function mountFileInspector(container, {api, sender, parentScope, variant function render(state) { const inner = el('div', undefined, full ? undefined : 'detail-inner'); if (state.loading || state.error) {container.append(inner); statusView(state, inner); return;} + // D124: "Load metadata on selection" is off, so the file's details wait for an explicit request. + if (state.deferred) { + container.append(inner); + const status = el('p', DEFERRED_TEXT, 'small'); status.setAttribute('role', 'status'); inner.append(status); + if (onLoad) {const load = createButton('Load metadata'); contentScope.listen(load, 'click', onLoad); inner.append(load);} + return; + } const record = state.record; if (!record) return; const {file, library} = record; @@ -326,8 +337,10 @@ export function mountFileInspector(container, {api, sender, parentScope, variant section.append(el('p', reported ? 'Current file at the reported location · identity unverified' : local ? 'Selected local file · read in this browser' : 'Current file at this source; historical bytes are not verified.', 'small')); const path = el('dd', record.identity.path ?? 'Not available'); + // System › Appearance "Show file paths" off: the Path row is left out; nothing else changes. + const showPath = !local && displayPreferences()?.showFilePaths !== false; const facts = file && definitionList([ - ['Source', file.source_label], ...(local ? [] : [['Path', path]]), ['Size', formatBytes(file.size)], + ['Source', file.source_label], ...(showPath ? [['Path', path]] : []), ['Size', formatBytes(file.size)], ['Dimensions', file.width && file.height ? `${file.width} × ${file.height}` : null], ['Format', file.format || `${extensionOf(record)} extension`], ['Media type', file.media_type], ['Modified', file.modified_time !== null ? new Date(file.modified_time * 1000).toLocaleString() : null], @@ -335,8 +348,9 @@ export function mountFileInspector(container, {api, sender, parentScope, variant if (facts) { // D38: "Show in folder" follows the Path value. const target = api ? revealTarget(record.identity) : null; - if (target) path.append(' ', revealButton(target)); section.append(facts); + if (target && showPath) path.append(' ', revealButton(target)); + else if (target) section.append(revealButton(target)); } else section.append(el('p', 'Current file observations are unavailable for this inspection response.', 'small')); // The concept's "Prompt evidence: execution recorded" line, in the D21 badge's words; no badge, no line. if (badge) section.append(el('p', `Prompt evidence: ${badge.toLowerCase()}`, 'small')); diff --git a/web/js/workspace/image-card.mjs b/web/js/workspace/image-card.mjs index dd71178..4d3f799 100644 --- a/web/js/workspace/image-card.mjs +++ b/web/js/workspace/image-card.mjs @@ -36,10 +36,10 @@ export function cardMedia(image, lazy = false) { /** * One card for `image`. `checked` (a boolean) adds the D8 selection checkbox, `favoriteBusy` paints a pending star - * (D7, shown wherever the image reports `favorite`), and `media: false` leaves the open button empty for a file that - * cannot be served. + * (D7, shown wherever the image reports `favorite`), `media: false` leaves the open button empty for a file that + * cannot be served, and `showInfo: false` hides the title and file line (D97). */ -export function createImageCard(image, { checked = null, favoriteBusy = false, media = true } = {}) { +export function createImageCard(image, { checked = null, favoriteBusy = false, media = true, showInfo = true } = {}) { const card = el('article', undefined, 'media-card'); card.dataset.imageKey = imageKey(image); card.setAttribute('role', 'listitem'); card.style.setProperty('--media-ratio', mediaRatio(image)); @@ -68,6 +68,8 @@ export function createImageCard(image, { checked = null, favoriteBusy = false, m const text = el('div'); const title = el('h3', cardTitle(image)); if (image.path) title.title = image.path; text.append(title, el('p', cardSubtitle(image))); + // D97: `showInfo` false (System › Appearance) hides the title and file line; Info and Tag stay. + if (!showInfo) text.hidden = true; const actions = el('div', undefined, 'image-card-actions'); const inspectButton = createButton('', 'icon-btn'); inspectButton.append(createWorkspaceIcon('info')); diff --git a/web/js/workspace/images-state.mjs b/web/js/workspace/images-state.mjs index d133eba..c61daf4 100644 --- a/web/js/workspace/images-state.mjs +++ b/web/js/workspace/images-state.mjs @@ -1,6 +1,17 @@ import { noun } from './plural.mjs'; /** Identity is the source and relative path; content hashes may be shared by files. */ +/** + * The layout and thumbnail size an Images visit starts with (#249 System S3). The last visit (the current view) keeps + * its own; a saved view starts as before, at Grid and Comfortable; any other new visit starts from System › + * Appearance's defaults (`preferences`, preferences.mjs values, or null when unavailable). + */ +export function initialImagesView({ prior = null, saved = null, preferences = null } = {}) { + if (prior) return { layout: prior.layout || 'grid', density: prior.density || 'comfortable' }; + if (saved || !preferences) return { layout: 'grid', density: 'comfortable' }; + return { layout: preferences.workspaceLayout || 'grid', density: preferences.workspaceDensity || 'comfortable' }; +} + export function imageKey(image) { return `${image.root_index}:${image.path}`; } diff --git a/web/js/workspace/images.mjs b/web/js/workspace/images.mjs index b4c0a88..c52bab6 100644 --- a/web/js/workspace/images.mjs +++ b/web/js/workspace/images.mjs @@ -1,5 +1,5 @@ import { createScope, createButton } from '../shared/toolkit/core.js'; -import { createImageStream, withItem, imageKey, countLine, captionDate, scopeLabel, activeRefinements } from './images-state.mjs'; +import { createImageStream, withItem, imageKey, countLine, captionDate, scopeLabel, activeRefinements, initialImagesView } from './images-state.mjs'; import { createImageCard } from './image-card.mjs'; import { layoutImages, visibleImages, gridMetrics, stacksCardInfo } from './image-layout.mjs'; import { mountFileInspector } from './file-inspector.mjs'; @@ -16,6 +16,7 @@ import { openImportDialog, openRefineDialog } from './workspace-dialogs.mjs'; import { openMetadataScan } from './activity-scan.mjs'; import { showWorkspaceToast } from './toast.mjs'; import { imagesFilters, imagesViewState } from './saved-views.mjs'; +import { displayPreferences } from './preferences.mjs'; import { onImageTagsSaved } from './image-tagging.mjs'; import { followImageTagSaves, openCardTagging } from './images-tagging.mjs'; import { favoriteBusy, paintStar, receiveFavorites, setFavoriteVisitTarget, toggleFavorite } from './images-favorites.mjs'; @@ -111,7 +112,9 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage if (parentScope.signal.aborted) { scope.dispose(); return {dispose}; } const sender = createPromptSender({scope}); let detailInspector; - const inspection = createInspectionController({api}); + // System › Appearance: the display defaults this visit starts from (preferences.mjs); null without gallery-settings.js. + const preferences = displayPreferences(); + const inspection = createInspectionController({api, autoLoad:preferences?.autoLoadMetadata !== false, cache:preferences?.cacheMetadata !== false}); scope.cleanup(inspection.subscribe(next => detailInspector?.setState(next))); scope.cleanup(() => inspection.dispose()); scope.cleanup(() => detailInspector?.dispose()); @@ -119,9 +122,12 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage const prior = embedded || saved || favorites ? null : lastVisit; // A stale visit keeps its filters, layout and size, and lists again instead of restoring (M6). const visit = prior?.stale ? null : prior; - let density = prior?.density || 'comfortable'; + // The layout and size defaults are the Images page's: an embedded gallery (a prompt history's, its dialogs) has no + // toolbar to change them, so it keeps its fixed Grid/Comfortable. The metadata and card defaults apply everywhere. + const start = initialImagesView({prior, saved, preferences:embedded ? null : preferences}); + let density = start.density; let metrics = {...INITIAL_METRICS}; - let layout = prior?.layout || 'grid', selection = visit?.selection || null, frame = 0, loadFrame = 0, restoreFrame = 0, observer; + let layout = start.layout, selection = visit?.selection || null, frame = 0, loadFrame = 0, restoreFrame = 0, observer; let state, geometry = { positions: [], columns: 1, height: 0, maxHeight: 0 }, galleryWidth = 0, optionsRequest, categoryRequest, recovering = false, rescanning = false; // Discovered sources (each with its own folders) and every folder, for "Refine this view" and the scope chip. let roots = [], folders = []; @@ -273,7 +279,7 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage selection = imageKey(image); for (const video of details.querySelectorAll('video')) releaseVideo(video); detailInspector = mountFileInspector(details, {sender, parentScope:scope, variant:'panel', - onTagFilter:embedded ? undefined : filterByTag, onRetry:() => inspection.retry(), onRefresh:refreshInspectionCollection}); + onTagFilter:embedded ? undefined : filterByTag, onRetry:() => inspection.retry(), onLoad:() => inspection.load(), onRefresh:refreshInspectionCollection}); detailInspector.setState(inspection.state); inspection.select(image); for (const [key, card] of rendered) markSelected(card, key === selection); @@ -289,7 +295,9 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage /** The concept media card (image-card.mjs): the image opens the viewer, Info inspects (D7–D11, D19). */ function cardFor(image) { const key = imageKey(image); - const card = createImageCard(image, {checked:bar ? bar.isChecked(key) : null, favoriteBusy:favoriteBusy(key)}); + const card = createImageCard(image, {checked:bar ? bar.isChecked(key) : null, favoriteBusy:favoriteBusy(key), + // D97: "Show image details on cards" off hides the title and file line; Info and Tag stay. + showInfo:preferences?.showImageInfo !== false}); markSelected(card, key === selection); return card; } diff --git a/web/js/workspace/inspection-state.mjs b/web/js/workspace/inspection-state.mjs index 290797b..f9567ed 100644 --- a/web/js/workspace/inspection-state.mjs +++ b/web/js/workspace/inspection-state.mjs @@ -344,9 +344,16 @@ export function createInspectionLoader(api, onChange, { extractFacts, adapter = }; } -const emptyState = () => ({ loading: false, error: false, record: null, prompts: undefined, message: '', refreshRequired: false, code: null, recovery: null }); +const emptyState = () => ({ loading: false, error: false, deferred: false, record: null, prompts: undefined, message: '', refreshRequired: false, code: null, recovery: null }); -export function createInspectionController({ api, extractFacts, adapter = rootedFileAdapter }) { +/** + * One selection's inspection. #249 System S3 (Appearance): `autoLoad: false` ("Load metadata on selection" off) makes a + * selection publish `deferred` instead of reading, and `load()` reads it once on request; `cache: false` ("Cache + * metadata in this browser" off) re-reads the selected file when it is selected again, where the default reuses the one + * current record. A read in flight is shared either way, and a file whose source changed is never re-read by selecting + * it again (as `retry()` and `refresh()` refuse). + */ +export function createInspectionController({ api, extractFacts, adapter = rootedFileAdapter, autoLoad = true, cache = true }) { let state = Object.freeze(emptyState()), current = null, pending = null, disposed = false; let attempted, hasAttempt = false; const listeners = new Set(); @@ -369,13 +376,19 @@ export function createInspectionController({ api, extractFacts, adapter = rooted try { adapter.validate(image); const scope = adapter.scope(image), key = adapter.key(image), path = adapter.inspectionPath(image); - if (current?.key === key && current.scope === scope && current.path === path) return pending; + if (current?.key === key && current.scope === scope && current.path === path && (cache || state.loading || state.deferred || state.refreshRequired)) return pending; selected = { key, scope, path }; } catch { // The loader publishes validation errors after invalidating the previous source. } loader.dispose(); loader = makeLoader(); current = selected; attempted = object(image) ? { ...image, ...(adapter !== rootedFileAdapter ? {occurrence:{...image.occurrence}, reference:{...image.reference}} : {}) } : image; hasAttempt = true; + if (!autoLoad && selected) { pending = null; publish({ deferred: true }); return undefined; } + return read(attempted); + }, + /** Read a deferred selection (the inspector's "Load metadata"); a read already made or running is reused. */ + load() { + if (disposed || !hasAttempt || !state.deferred) return pending ?? undefined; return read(attempted); }, retry() { @@ -384,7 +397,8 @@ export function createInspectionController({ api, extractFacts, adapter = rooted }, /** Re-read the selected file keeping the shown record until the answer (a retry when none is shown). */ refresh() { - if (disposed || !hasAttempt || state.refreshRequired) return; + // A deferred selection loads only through `load()` (the user's "Load metadata"). + if (disposed || !hasAttempt || state.refreshRequired || state.deferred) return; return read(attempted, { quiet: Boolean(state.record) && !state.loading && !state.error }); }, clear() { diff --git a/web/js/workspace/media-viewer.mjs b/web/js/workspace/media-viewer.mjs index bd1b323..834c995 100644 --- a/web/js/workspace/media-viewer.mjs +++ b/web/js/workspace/media-viewer.mjs @@ -2,14 +2,14 @@ import {createScope, createButton} from '../shared/toolkit/core.js'; import {createViewerSession} from './viewer-state.mjs'; import {rootedFileAdapter} from './inspection-state.mjs'; import {mountFileInspector} from './file-inspector.mjs'; +import {displayPreferences, readPreferences} from './preferences.mjs'; let viewerId = 0; // Upper bound on waiting for a closed Viewer's in-flight image view before destroying it. const PENDING_VIEW_DESTROY_MS = 10000; +/** The video defaults, read through the shared display defaults (preferences.mjs, #249 System S3). */ export function readViewerPreferences(getStorage = () => window.localStorage, normalize = globalThis.GallerySettings.normalizeGallerySettings) { - let stored; - try { stored = JSON.parse(getStorage().getItem('gallerySettings')); } catch { stored = null; } - const {videoAutoplay,videoMute,videoLoop} = normalize(stored); + const {videoAutoplay,videoMute,videoLoop} = readPreferences(getStorage,normalize).values; return {videoAutoplay,videoMute,videoLoop}; } function element(tag, className, text) { @@ -74,15 +74,31 @@ export function createMediaViewer({api,container,parentScope,inspectionControlle local.listen(download,'click',event => {session.stop();if (download.getAttribute('aria-disabled') === 'true') event.preventDefault();}); controls.append(download); const next = button('Next media','Next →',() => session.navigate(1)); + // D97: "Collapse viewer details by default" opens the viewer with its details hidden behind one toggle. + const collapsed = displayPreferences()?.sidebarCollapsedByDefault === true; + let detailsToggle = null; + if (collapsed) { + detailsToggle = createButton('Show details');detailsToggle.setAttribute('aria-expanded','false'); + detailsToggle.setAttribute('aria-controls','viewer-details');controls.append(detailsToggle); + } const position = element('p','media-position muted');position.id = `media-viewer-position-${++viewerId}`; position.setAttribute('role','status'); const reason = element('p','media-reason muted');reason.id = `media-viewer-reason-${viewerId}`; const slideshowReason = element('p','media-slideshow-reason muted');slideshowReason.id = `media-viewer-slideshow-${viewerId}`; controls.append(position,reason,slideshowReason);left.append(stage,controls); const details = element('aside');details.id = 'viewer-details';details.setAttribute('aria-label','Image metadata'); + if (detailsToggle) { + details.hidden = true; + local.listen(detailsToggle,'click',() => { + details.hidden = !details.hidden; + detailsToggle.textContent = details.hidden ? 'Show details' : 'Hide details'; + detailsToggle.setAttribute('aria-expanded',String(!details.hidden)); + }); + } body.append(left,details);dialog.append(header,body);container.append(dialog); const localSender = {open(roles,trigger) {session.stop();sender.open(roles,trigger);}}; const inspector = mountFileInspector(details,{api,sender:localSender,parentScope:local,variant:'full',onInteraction:() => session.stop(),onRetry:() => inspectionController.retry(), + onLoad:() => inspectionController.load?.(), onRefresh:onRefresh ? () => { closeViewer();onRefresh(); } : undefined}); function closeViewer(restore = true) { if (closed) return; diff --git a/web/js/workspace/preferences.mjs b/web/js/workspace/preferences.mjs new file mode 100644 index 0000000..c964d84 --- /dev/null +++ b/web/js/workspace/preferences.mjs @@ -0,0 +1,112 @@ +/** + * Browser display defaults (#249 System S3 Task 7). The workspace keeps them in the legacy gallery's own + * `gallerySettings` object (Q3, "One display configuration"), so the gallery, the workspace and the viewer read one + * configuration: the new `workspaceLayout` and `workspaceDensity` beside the legacy keys they share (metadata, paths, + * card details, viewer details, image quality and the video defaults). The legacy paging keys (`defaultLimit`, + * `gridColumns`, `infiniteScroll`, …) belong to the legacy gallery and are never read here (README continuous-browsing + * ruling). + * + * - `readPreferences(getStorage, normalize)` → `{values, persisted}`: the workspace keys, normalized by `gallery-settings.js`; + * `persisted` is false when this browser refused its storage. + * - `writePreferences(dirty, getStorage, {baseline})` → `{persisted, conflicts}`: re-reads the stored object and writes + * back only the dirty keys, every other key byte-for-byte as stored. A write the browser refused (denied storage, a + * full quota) leaves the stored bytes as they were and keeps the change for this tab only, so every reader in this + * tab still sees it; the next write storage accepts stores those changes too. `conflicts` lists the dirty keys another page changed since `baseline` was read; the explicit + * save still wins. + * - `subscribePreferences(win, onChange, getStorage)` → unsubscribe: `onChange(readPreferences())` after another page + * changed the settings (a `storage` event). + * - `reconcilePreferences({baseline, drafts, next})` → `{conflicts}`: which drafts such a change overtook. + * + * Every storage access is in try/catch: a private window, blocked site data or a malformed value never breaks a page. + */ +export const PREFERENCES_KEY = 'gallerySettings'; +export const WORKSPACE_PREFERENCE_KEYS = Object.freeze(['workspaceLayout', 'workspaceDensity', 'autoLoadMetadata', 'cacheMetadata', + 'imageQuality', 'videoAutoplay', 'videoMute', 'videoLoop', 'showFilePaths', 'showImageInfo', 'sidebarCollapsedByDefault']); + +// The changes this tab could not store, overlaid on every read until storage accepts them. +const tabOnly = new Map(); +const defaultStorage = () => globalThis.localStorage; +const normalizer = () => globalThis.GallerySettings.normalizeGallerySettings; +const isObject = value => value !== null && typeof value === 'object' && !Array.isArray(value); + +/** Forget what this tab kept without storage (tests, and nothing else). */ +export function resetTabPreferences() { + tabOnly.clear(); +} + +const pick = settings => Object.fromEntries(WORKSPACE_PREFERENCE_KEYS.map(key => [key, settings[key]])); +/** The gallery's defaults for the workspace keys. */ +export function preferenceDefaults() { + return pick(normalizer()(null)); +} + +/** The stored object (`{}` when absent or malformed), or null when storage is unavailable. */ +function storedObject(storage) { + let text; + try { text = storage.getItem(PREFERENCES_KEY); } catch { return null; } + try { + const parsed = text ? JSON.parse(text) : null; + return isObject(parsed) ? parsed : {}; + } catch { + return {}; + } +} + +function storageOf(getStorage) { + try { return getStorage() ?? null; } catch { return null; } +} + +/** `normalize` is gallery-settings.js's `normalizeGallerySettings` (the page's `window.GallerySettings` by default). */ +export function readPreferences(getStorage = defaultStorage, normalize = normalizer()) { + const storage = storageOf(getStorage); + const stored = storage ? storedObject(storage) : null; + const values = pick(normalize({ ...(stored ?? {}), ...Object.fromEntries(tabOnly) })); + return { values, persisted: stored !== null }; +} + +/** A change is only a workspace key with a value the gallery's normalizer keeps as given. */ +function validate(dirty) { + if (!isObject(dirty)) throw new TypeError('Preferences need an object of changes'); + for (const [key, value] of Object.entries(dirty)) { + if (!WORKSPACE_PREFERENCE_KEYS.includes(key)) throw new TypeError(`Not a workspace display preference: ${key}`); + if (normalizer()({ [key]: value })[key] !== value) throw new TypeError(`Invalid value for ${key}`); + } +} + +export function writePreferences(dirty, getStorage = defaultStorage, { baseline = null } = {}) { + validate(dirty); + const keep = () => { for (const [key, value] of Object.entries(dirty)) tabOnly.set(key, value); }; + const storage = storageOf(getStorage); + const stored = storage ? storedObject(storage) : null; + if (stored === null) { keep(); return { persisted: false, conflicts: [] }; } + const current = pick(normalizer()(stored)); + const conflicts = baseline ? Object.keys(dirty).filter(key => Object.hasOwn(baseline, key) && current[key] !== baseline[key]) : []; + try { + // The changes this tab could not store before are stored with this one, so "Saved" is true for all of them. + storage.setItem(PREFERENCES_KEY, JSON.stringify({ ...stored, ...Object.fromEntries(tabOnly), ...dirty })); + } catch { + keep(); + return { persisted: false, conflicts }; + } + tabOnly.clear(); + return { persisted: true, conflicts }; +} + +/** The current values for a display consumer, or null on a page without gallery-settings.js (a unit test). */ +export function displayPreferences(getStorage = defaultStorage) { + try { return readPreferences(getStorage).values; } catch { return null; } +} + +export function subscribePreferences(win, onChange, getStorage = defaultStorage) { + const listener = event => { + if (event.key !== null && event.key !== PREFERENCES_KEY) return; + if (event.storageArea && event.storageArea !== storageOf(getStorage)) return; + onChange(readPreferences(getStorage)); + }; + win.addEventListener('storage', listener); + return () => win.removeEventListener('storage', listener); +} + +export function reconcilePreferences({ baseline, drafts, next }) { + return { conflicts: Object.keys(drafts).filter(key => next[key] !== baseline[key] && next[key] !== drafts[key]) }; +} diff --git a/web/js/workspace/system-appearance.mjs b/web/js/workspace/system-appearance.mjs new file mode 100644 index 0000000..290bf77 --- /dev/null +++ b/web/js/workspace/system-appearance.mjs @@ -0,0 +1,143 @@ +/** + * System › Appearance (#249 System S3 Task 8): the concept's `renderSystem` "appearance" body (app.js 676–678), one + * `section.surface` "Browsing & display" of `.setting-row`s whose values are the drafts of the one Save row. + * + * - The browser rows are the display defaults this browser keeps in the shared `gallerySettings` object + * (preferences.mjs, Q3): Default layout and Thumbnail size (`workspaceLayout`, `workspaceDensity`), the metadata, + * quality (Optimized→`medium`, Original→`high`, Fast→`low`), video and path rows. Images, the inspector and the + * viewer read them on their next visit; a toolbar change still applies to the current view only. + * - "Open from ComfyUI" (New tab→`newtab`, Popup window→`popup`) and "Node search auto-hide (seconds)" (a whole number + * 0–300) are the server's `webui_display_mode` and `result_timeout`, saved with the settings revision (#220). + * - "Continuous browsing" is the concept's fixed row: there is no paging or column control (README ruling). + * - D97: added rows for the legacy gallery settings the concept leaves out: "Show image details on cards" + * (`showImageInfo`), "Collapse viewer details by default" (`sidebarCollapsedByDefault`) and the operation "Reset + * display defaults", which drafts the gallery's defaults for these browser rows and touches no other key. + * + * Controls follow the store: a value read later (the host settings, another tab's change) repaints them. A host row + * whose value could not be read stays disabled and says so ("Server settings could not be read."). + */ +import { settingRow, operationRow } from './system-controls.mjs'; +import { preferenceDefaults } from './preferences.mjs'; + +export const RESULT_TIMEOUT_MAX = 300; +/** A host row's line while the server settings could not be read (its control stays disabled). */ +export const HOST_UNAVAILABLE = 'Server settings could not be read.'; +const BOOLEAN = Object.freeze({ type: 'toggle' }); + +/** + * The concept's rows in its order, then the D97 rows: `[id, label, description, domain, key, control]`. A select's + * options are `[stored value, concept label]`. + */ +export const APPEARANCE_ROWS = Object.freeze([ + ['default-view', 'Default layout', 'Choose your starting image layout.', 'browser', 'workspaceLayout', + { type: 'select', options: [['grid', 'Grid'], ['list', 'List'], ['masonry', 'Masonry']] }], + ['density', 'Thumbnail size', 'Readable previews by default; no fixed column-count setting.', 'browser', 'workspaceDensity', + { type: 'select', options: [['comfortable', 'Comfortable'], ['large', 'Large'], ['compact', 'Compact']] }], + 'continuous', + ['metadata-auto', 'Load metadata on selection', 'Use the same inspector in the gallery and image viewer.', 'browser', 'autoLoadMetadata', BOOLEAN], + ['metadata-cache', 'Cache metadata in this browser', 'Reuse fetched metadata until it changes or you clear this cache.', 'browser', 'cacheMetadata', BOOLEAN], + ['image-quality', 'Image loading quality', 'Preserve original / optimized / fast image-loading preferences.', 'browser', 'imageQuality', + { type: 'select', options: [['medium', 'Optimized'], ['high', 'Original'], ['low', 'Fast']] }], + ['video-auto', 'Autoplay videos', 'Videos stay within the same gallery and use native playback controls.', 'browser', 'videoAutoplay', BOOLEAN], + ['video-muted', 'Mute videos by default', 'Keep playback quiet while browsing.', 'browser', 'videoMute', BOOLEAN], + ['video-loop', 'Loop videos', 'Repeat a selected clip until you navigate away.', 'browser', 'videoLoop', BOOLEAN], + ['paths', 'Show file paths', 'Paths appear under File & provenance, not across every card.', 'browser', 'showFilePaths', BOOLEAN], + ['open-mode', 'Open from ComfyUI', 'Keep the node pack’s popup / new-tab preference.', 'host', 'webui_display_mode', + { type: 'select', options: [['newtab', 'New tab'], ['popup', 'Popup window']] }], + ['timeout', 'Node search auto-hide (seconds)', 'Node search behavior only. Use 0 to keep results visible.', 'host', 'result_timeout', + { type: 'number' }], + // D97: the legacy gallery's card and viewer settings. + ['card-details', 'Show image details on cards', 'Show each card’s title and file details under the preview.', 'browser', 'showImageInfo', BOOLEAN], + ['viewer-details', 'Collapse viewer details by default', 'Open the image viewer with its details panel hidden.', 'browser', + 'sidebarCollapsedByDefault', BOOLEAN], +]); + +/** The browser keys "Reset display defaults" restores (D97). */ +export const DISPLAY_KEYS = Object.freeze(APPEARANCE_ROWS.filter(row => Array.isArray(row) && row[3] === 'browser').map(row => row[4])); + +/** A whole number of seconds from 0 to 300 (the server's `validate_result_timeout`), else null. */ +export function resultTimeout(text) { + const value = String(text).trim(); + if (!/^\d+$/.test(value)) return null; + const seconds = Number(value); + return seconds <= RESULT_TIMEOUT_MAX ? seconds : null; +} + +function el(tag, text, className) { + const node = document.createElement(tag); + if (text !== undefined) node.textContent = text; + if (className) node.className = className; + return node; +} + +/** The concept's fixed "Continuous browsing" row: `div > h3 + p` and the "Infinite scroll" chip. */ +function continuousRow() { + const row = el('div', undefined, 'setting-row'); + const text = el('div'); + text.append(el('h3', 'Continuous browsing'), + el('p', 'Images load automatically as you scroll in Grid, List and Masonry. Batch sizing is handled internally.')); + row.append(text, el('span', 'Infinite scroll', 'chip blue')); + return row; +} + +/** The row's control (`data-setting`), wherever the Toolkit switch nests it. */ +function controlOf(node, id) { + if (node.dataset?.setting === id) return node; + for (const child of node.children ?? []) { + const found = controlOf(child, id); + if (found) return found; + } + return null; +} + +/** Show a stored value in its control; a value not read yet leaves the control disabled. */ +function paint(input, control, value) { + input.disabled = value === undefined; + if (value === undefined) return; + if (control.type === 'toggle') input.checked = value === true; + else if (document.activeElement !== input || control.type === 'select') input.value = String(value); +} + +/** Mount Appearance in `container` (System's `.settings-layout`); returns the section's nodes. */ +export function mountAppearance(container, { scope, drafts }) { + const surface = el('section', undefined, 'surface'); + surface.append(el('h2', 'Browsing & display'), el('p', 'One set of defaults; toolbar changes apply to the current view.')); + const controls = []; + for (const row of APPEARANCE_ROWS) { + if (row === 'continuous') { surface.append(continuousRow()); continue; } + const [id, label, description, domain, key, control] = row; + const onChange = value => { + if (key === 'result_timeout') { + const seconds = resultTimeout(input.value); + if (seconds === null) { drafts.reject(domain, key); return; } + value = seconds; + } + drafts.set(domain, key, value); + }; + const node = settingRow({ id, label, description, control, scope, onChange }); + surface.append(node); + const input = controlOf(node, id); + input.dataset.key = key; + controls.push({ input, control, domain, key, text: node.children[0], note: null }); + } + surface.append(operationRow({ title: 'Reset display defaults', + description: 'Restore this browser’s display defaults, then save. Other settings are kept.', label: 'Reset', scope, + onClick: () => { + const defaults = preferenceDefaults(); + for (const key of DISPLAY_KEYS) drafts.set('browser', key, defaults[key]); + } })); + const repaint = () => { + const unavailable = drafts.hostState?.() === 'failed'; + for (const row of controls) { + const value = drafts.get(row.domain, row.key); + paint(row.input, row.control, value); + if (row.domain !== 'host') continue; + if (unavailable && value === undefined) { + if (!row.note) { row.note = el('p', HOST_UNAVAILABLE, 'small'); row.text.append(row.note); } + } else if (row.note) { row.note.remove(); row.note = null; } + } + }; + scope.cleanup(drafts.subscribe(repaint)); + repaint(); + return [surface]; +} diff --git a/web/js/workspace/system-state.mjs b/web/js/workspace/system-state.mjs index 2c24e67..b827dc3 100644 --- a/web/js/workspace/system-state.mjs +++ b/web/js/workspace/system-state.mjs @@ -15,8 +15,9 @@ * the page session (Q10a): a section left and opened again shows them, and a reload of its values keeps them. * * Adapters: `readBrowser()` and `readRuntime()` return the values; `readHost()` returns `{values, revision}`. - * `writeBrowser(changes)` returns `{persisted}` (false: kept for this tab only), `writeHost(changes, - * {expected_revision})` the settings response (`{success, persisted, revision}`), `writeRuntime(changes)` anything; + * `writeBrowser(changes)` returns `{persisted}` (false: kept for this tab only) and throws an error with `code: + * 'settings-changed'` when another page changed a drafted value (a conflict: the browser values are read again), + * `writeHost(changes, {expected_revision})` the settings response (`{success, persisted, revision}`), `writeRuntime(changes)` anything; * a write that throws failed. */ export { SYSTEM_SECTIONS, isSystemSection } from './state.mjs'; @@ -64,6 +65,7 @@ export function createSystemDrafts(adapters) { const failedKeys = { browser: new Set(), host: new Set(), runtime: new Set() }; const listeners = new Set(); let revision = null; + let hostRead = 'pending'; let outcome = null; let saving = null; @@ -78,6 +80,7 @@ export function createSystemDrafts(adapters) { const read = await adapters.readHost(); values = read?.values ?? {}; revision = read?.revision ?? null; + hostRead = 'loaded'; } else { values = await (domain === 'browser' ? adapters.readBrowser() : adapters.readRuntime()); } @@ -159,8 +162,11 @@ export function createSystemDrafts(adapters) { } } - /** Read the host values again; a failed read leaves host saves refused until one succeeds. */ - const reloadHost = () => load('host').then(() => true, () => false); + /** + * Read the host values again; a failed read leaves host saves refused until one succeeds, and `hostState()` says + * so (System S3: the host rows show that the server settings could not be read). + */ + const reloadHost = () => load('host').then(() => true, () => { hostRead = 'failed'; notify(); return false; }); /** Save the dirty domains in order; a Save pressed while one runs joins it. */ function save() { @@ -170,6 +176,7 @@ export function createSystemDrafts(adapters) { for (const domain of DRAFT_DOMAINS) result[domain] = await saveDomain(domain); // Settings changed elsewhere: read them (and their revision), keeping the drafts (Q10). if (result.host === 'conflict') await reloadHost(); + if (result.browser === 'conflict') await load('browser').catch(() => {}); return result; })().then(result => { saving = null; @@ -180,9 +187,39 @@ export function createSystemDrafts(adapters) { return saving; } + /** + * A value the control could not take (System S3: a timeout outside 0–300): no draft, the control is highlighted and + * the status says a change was not saved. + */ + function reject(domain, key) { + requireDomain(domain); + drafts[domain].delete(key); + failedKeys[domain].add(key); + outcome = { [domain]: 'failed' }; + notify(); + } + + /** + * Another page changed saved values this tab has drafts for (a browser `storage` event, System S3): the drafts stay + * and are highlighted, and the status says the settings changed elsewhere. + */ + function conflict(domain, keys) { + requireDomain(domain); + if (!keys.length) return; + for (const key of keys) failedKeys[domain].add(key); + outcome = { [domain]: 'conflict' }; + notify(); + } + return { load, reloadHost, + /** Whether the host values were read: `pending`, `loaded`, or `failed` (the last read failed). */ + hostState: () => hostRead, + reject, + conflict, + /** The saved values of a domain (its last read or save), not the drafts. */ + saved: domain => { requireDomain(domain); return { ...baseline[domain] }; }, get, set, dirty, diff --git a/web/js/workspace/system.mjs b/web/js/workspace/system.mjs index 0fa7658..27e1a54 100644 --- a/web/js/workspace/system.mjs +++ b/web/js/workspace/system.mjs @@ -4,7 +4,8 @@ * `aria-pressed`; a press navigates to its hash), then `.settings-layout` with the section's body and the Save row * (`span.small#settings-status` + "Save preferences"). D92: no "Sample configuration" chip, the status copy is * truthful and "Save preferences" is aria-disabled while nothing is unsaved. A section not delivered yet shows the - * per-section hand-off to its legacy tool (D94), never mock placeholders. Overview is native (system-overview.mjs). + * per-section hand-off to its legacy tool (D94), never mock placeholders. Overview (system-overview.mjs) and + * Appearance (system-appearance.mjs) are native. * * Switching sections keeps the shell (and focus on the pressed button) and disposes the old section's scope, so its * listeners and timers never outlive it. The drafts are module-level (Q10): they live for the page session. @@ -14,6 +15,8 @@ import { SYSTEM_SECTIONS, navigateTo } from './state.mjs'; import { createSystemDrafts, DRAFT_DOMAINS } from './system-state.mjs'; import { sectionNav } from './system-controls.mjs'; import { healthNotice, mountOverview } from './system-overview.mjs'; +import { mountAppearance } from './system-appearance.mjs'; +import { displayPreferences, readPreferences, reconcilePreferences, subscribePreferences, writePreferences } from './preferences.mjs'; const SETTINGS_URL = '/prompt_manager/settings'; const ADMIN = Object.freeze({ href: '/prompt_manager/admin', label: 'Open existing administration' }); @@ -24,7 +27,6 @@ const GALLERY = Object.freeze({ href: '/prompt_manager/gallery', label: 'Open ex * settings, thumbnails, duplicates and cache tools. */ export const SECTION_HANDOFFS = Object.freeze({ - appearance: [GALLERY, ADMIN], sources: [ADMIN, GALLERY], integrations: [ADMIN], tagging: [ADMIN], @@ -46,12 +48,23 @@ function el(tag, text, className) { /** A domain whose settings no delivered section edits yet: nothing can be drafted, and a write would say it failed. */ const undelivered = async () => { throw new Error(NOT_DELIVERED); }; -/** The page session's drafts (Q10); the host domain reads and saves the server settings with their revision (#220). */ +/** + * The page session's drafts (Q10). The browser domain is this browser's display defaults (preferences.mjs): a write + * says whether storage kept it, so the status never claims "Saved in this browser." for a change only this tab holds. + * The host domain reads and saves the server settings with their revision (#220). + */ export function systemDrafts(api) { drafts ??= createSystemDrafts({ - readBrowser: () => ({}), + readBrowser: () => readPreferences().values, readRuntime: () => ({}), - writeBrowser: undelivered, + writeBrowser: changes => { + // Another page changed a drafted value since this tab read it, and no storage event told it yet: refuse, + // so the store keeps the draft, flags it and reads the values again; the next Save is an explicit choice. + const baseline = drafts.saved('browser'); + const { conflicts } = reconcilePreferences({ baseline, drafts: changes, next: readPreferences().values }); + if (conflicts.length) throw Object.assign(new Error('Settings changed elsewhere'), { code: 'settings-changed' }); + return { persisted: writePreferences(changes, undefined, { baseline }).persisted }; + }, writeRuntime: undelivered, async readHost() { const result = await api.get(SETTINGS_URL, { cache: 'no-store' }); @@ -80,17 +93,19 @@ function handoff(section) { function markFailed(layout, store) { const failed = new Set(DRAFT_DOMAINS.flatMap(domain => [...store.failed(domain)])); for (const control of layout.querySelectorAll('[data-setting]')) { - if (failed.has(control.dataset.setting)) control.setAttribute('aria-invalid', 'true'); + // A row's control names its stored key when that differs from the row id (Appearance's `data-key`). + if (failed.has(control.dataset.key ?? control.dataset.setting)) control.setAttribute('aria-invalid', 'true'); else control.removeAttribute('aria-invalid'); } } /** - * The sections delivered so far: Overview (S2 Task 6), and Maintenance's notice with "Run health checks" above the + * The sections delivered so far: Overview (S2 Task 6), Appearance (S3 Task 8), and Maintenance's notice with "Run health checks" above the * hand-off to the maintenance tools that are not native yet (D123). */ export const DELIVERED_SECTIONS = Object.freeze({ overview: mountOverview, + appearance: mountAppearance, maintenance: (container, { api, activity, scope }) => [healthNotice({ api, activity, scope }), ...handoff('maintenance')], }); @@ -101,11 +116,25 @@ export const DELIVERED_SECTIONS = Object.freeze({ * Activity store, which follows the jobs a section starts. * * The host values are read on mount (and by the store again after a conflict), so a save carries the current revision; - * until a read returned one, a host save fails without a request (system-state.mjs). + * until a read returned one, a host save fails without a request (system-state.mjs). The browser values are read on + * mount and again whenever another page changes them (a `storage` event). Either way a value another page changed + * under one of this tab's drafts flags that draft (D92's conflict status), including a change made while System was + * not mounted (the drafts outlive it, Q10). */ export function mountSystem(main, api, scope, { section, activity, sections = DELIVERED_SECTIONS, navigate = navigateTo }) { const store = systemDrafts(api); store.reloadHost(); + // Another tab or the legacy gallery changed the display defaults: clean values follow it; a draft it overtook stays + // and is flagged (D92's conflict status). `next` is null when the values cannot be read (no gallery-settings.js). + const reconcile = next => { + const baseline = store.saved('browser'); + // The first mount has read nothing yet, so nothing can have been overtaken. + const conflicts = next && Object.keys(baseline).length + ? reconcilePreferences({ baseline, drafts: store.changes('browser'), next }).conflicts : []; + store.load('browser').then(() => store.conflict('browser', conflicts), () => {}); + }; + reconcile(displayPreferences()); + scope.cleanup(subscribePreferences(window, next => reconcile(next.values))); const nav = sectionNav({ label: 'System sections', sections: SYSTEM_SECTIONS, active: section, scope, onSelect: id => navigate(`#/system/${id}`) }); const layout = el('div', undefined, 'settings-layout'); diff --git a/web/js/workspace/workspace.css b/web/js/workspace/workspace.css index 1727623..c7985b8 100644 --- a/web/js/workspace/workspace.css +++ b/web/js/workspace/workspace.css @@ -91,6 +91,10 @@ main:focus-visible { outline: 2px solid var(--ca-focus); outline-offset: 4px; } .setting-row [aria-invalid="true"] { outline: 2px solid var(--red); outline-offset: 2px; /* D92 */ } /* D93: an Overview row whose action is not available yet (the review queue, Q6) keeps its button, disabled. */ .operation .row-actions button[aria-disabled="true"] { opacity: .55; cursor: not-allowed; /* D93 */ } +/* D97: Appearance's legacy display rows. "Show image details on cards" off keeps a card's Info and Tag at the end of + its row; "Collapse viewer details by default" gives the viewer's stage the whole width while its details are hidden. */ +.media-info > div[hidden] + .image-card-actions { margin-inline-start: auto; /* D97 */ } +.viewer-layout:has(> #viewer-details[hidden]) { grid-template-columns: minmax(0, 1fr); /* D97 */ } @media (max-width: 700px) { .stat-grid { gap: 8px; } .stat { padding: 16px; }