fix(annotations): final review round for bulk image tagging

- Rename the D117 copy deviation to D118 (D117 belongs to another PR).
- The tagging job's progress line gives the scan's "about X min left"
  estimate, with each finished file as one batch.
- The start body cap fits 500 targets of the longest legal path, even
  escaped; one byte over answers 413.
- A capture store at capacity is retried with a short backoff, then the
  file is skipped with a rerun sentence instead of a Tag-dialog error.
- Output roots and their labels are resolved once per job in the
  executor, not twice per file on the event loop.
- Review each image holds Stop reviewing, Escape and x while a save is in
  flight, and its primary reads "Save and next" when only the manual
  field would be saved.
- An Add tags field that parses to no names keeps the dialog and its
  text and reports the message on the field.
- Image tagging finish toasts carry the counts ("... finished · N tagged
  · M errors").
- Specs: the bulk spec waits for the answered Start before checking that
  the invalid press sent nothing; the Activity spec uses a real error row.
This commit is contained in:
Vito Sansevero
2026-10-22 18:25:20 -07:00
parent 0faeceb453
commit 0ce62d7f0a
13 changed files with 442 additions and 99 deletions
+30 -13
View File
@@ -1208,13 +1208,18 @@ The Images selection bar shows the concept's **Add tags** after the count, then
**Auto-tag selected** (D85), before Set category. Both take the checked files when
pressed, in listing order, as their reveal identity (`{root_index, root_scope, path}`);
more than 500 toasts "Select at most 500 images." and opens nothing. The selection
stays after every start, refusal and review.
stays after every start, refusal and review. The job's start body is bounded so 500
targets of the longest legal path (1024 code points, even escaped as `\uXXXX`) fit:
`500 × (1024 × 6 + 256) + 64 KiB`; a larger body answers 413
(`tests/test_image_tag_jobs.py` "test_start_body_bounds").
- **Add tags** opens the concept's `simpleInput` (app.js a1378–1383) as Set category
does: "Tag selected images", "Tags, separated by commas" (empty: the concept's
"cinematic, concept" is sample text), Cancel / Apply. The names are parsed as the Tag
image dialog's manual field parses them (trimmed, case-insensitive repeats dropped,
first spelling wins); none toasts "Enter at least one tag.". Apply starts the
first spelling wins); a field that parses to no names keeps the dialog open with the
typed text and reports "Enter at least one tag." on the field (its native validity,
cleared on the next edit), and nothing is sent. Apply starts the
`image-tags` job (`POST /prompt_manager/jobs/image-tags`, `{mode: "add", targets,
tags}`) and toasts "Tagging started."; Activity shows its progress.
- A refused start toasts why and changes nothing: another record-changing job (a scan)
@@ -1249,16 +1254,20 @@ stays after every start, refusal and review.
running; its actions are Stop reviewing, Skip and Approve and next (Approve and
finish on the last). Approve saves the manual field plus the remaining chips as Save
does, without its per-image toast, and opens the next image; Skip writes nothing and
advances; Stop reviewing, Escape and × end the session. Each image is one gated
advances; Stop reviewing, Escape and × end the session. While Approve's save is in
flight, Skip and Stop reviewing are `aria-disabled` and Escape and × do nothing, so the
closing count includes that save. When no suggestions are on screen (a model change
cleared them, or Generate was refused) the primary saves the manual field only and
reads "Save and next" ("Save and finish" on the last image). Each image is one gated
`suggest` call, so the session holds no model between images; a `model-busy` answer
stays on that image with its sentence in `#tag-generation-status` and D116's Retry
until the user retries or skips. Ending toasts "<n> of <M> images tagged." and
returns focus to Auto-tag selected.
- **D91** extends to the bulk dialog: its Suggestions field is the job's and the
review's `suggestion_limit`.
- **D117** (copy the concept has no line for; the concept's `bulk-apply` only toasts a
demo sentence): an Add tags field that parses to no names toasts "Enter at least one
tag."; a start refused for no coded reason toasts "Tagging could not start. Try
- **D118** (copy the concept has no line for; the concept's `bulk-apply` only toasts a
demo sentence): an Add tags field that parses to no names reports "Enter at least one
tag." on the field; a start refused for no coded reason toasts "Tagging could not start. Try
again." or "Auto-tagging could not start. Try again." (as D55's "The scan could not
start. Try again."); and the bulk dialog's model panel is headed "Auto-tag <N>
image(s)" where the Tag image dialog's reads "Auto-tag this image". Everything else
@@ -1286,12 +1295,18 @@ stays after every start, refusal and review.
- The log is "● Tagging log — raw detail, click to show".
- The row and log wording is the job's (D1), since neither the concept nor the
mockup has a line for it: "Added N tag(s)", "No new tags", and the skip reasons
"Video: manual tags only", "Too large to verify", "File changed", "File not found"
and "Not a supported still image". Log prefixes are `+` tagged, `=` unchanged, `-`
skipped and `!` error.
"Video: manual tags only", "Too large to verify", "File changed", "File not found",
"Not a supported still image" and "The file could not be read now. Run the job again
for it." (the capture store stayed at capacity, for example with Tag dialogs open in
other tabs, after a short backoff of retries). Log prefixes are `+` tagged, `=`
unchanged, `-` skipped and `!` error.
- The progress line is the scan's: "Processing file N of M · about X min left", the
estimate (D56's `_time_left`) shown once two files are done, each file one batch.
- Download report saves `prompt-manager-image-tags-<YYYYMMDD-HHMM>.json`.
- The toasts stay the generic ones: "Tagging started." / "Auto-tagging started."
(D85) and "<title> finished.", "<title> was cancelled." or "<title> failed.".
- The start toasts are "Tagging started." / "Auto-tagging started." (D85); the end
toasts follow the design's pattern with the job's counts, as the scan's: "<title>
finished · N tagged · M errors", "<title> cancelled · N tagged" or "<title> failed ·
N tagged · M errors".
- Tests: `tests/js/workspace-activity-store.test.js` ("image-tags jobs get their own
tiles outcomes and pill verb", "unknown kinds keep the generic view");
`tests/browser/workspace-activity.spec.mjs` ("an image tagging job shows Tagged
@@ -1308,7 +1323,9 @@ selected with Apply automatically runs a job with the chosen model, thresholds a
suggestion count", "a model that is not downloaded keeps Start disabled and says
why", "Suggestions field prefills from the setting and caps chips at the entered
count", "Review each image walks the selection one dialog at a time and only approved
images gain tags" and "a running scan makes the start report busy". Units:
images gain tags", "Review holds Stop reviewing, Escape and × while a save is in
flight, and names a manual-only save" and "a running scan makes the start report
busy". Units:
`tests/js/workspace-images-tagging-bulk.test.js`,
`tests/js/workspace-images-tagging-review.test.js`.
@@ -1318,7 +1335,7 @@ images gain tags" and "a running scan makes the start report busy". Units:
| Auto-tag selected and its dialog | D85 (ruling L1) |
| How to apply and the Review each image session | D90 (ruling T3-bulk) |
| Suggestions count in the bulk dialog | D91 |
| Copy without a concept line: no tags, start fallbacks, the panel heading | D117 |
| Copy without a concept line: no tags, start fallbacks, the panel heading | D118 |
| Activity item and job view for image tagging jobs | D89 (plan D66) |
Native image tagging (#249 PR B routes, PR C UI): `POST
+13 -4
View File
@@ -198,13 +198,22 @@ def _plural(count, word):
return f"{count} {word}" if count == 1 else f"{count} {word}s"
def _time_left(done, total, batches, elapsed):
""" " · about X min left" once enough batches ran, else "" (D56).
Shared by every Activity job that reports "Processing file N of M": the
scan counts its batches, the image tagging job each file.
"""
if batches < MIN_BATCHES_FOR_ESTIMATE or done <= 0 or done >= total:
return ""
minutes = max(1, math.ceil(elapsed / done * (total - done) / 60))
return f" · about {minutes} min left"
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:,}"
if batches < MIN_BATCHES_FOR_ESTIMATE or done <= 0 or done >= total:
return line
minutes = max(1, math.ceil(elapsed / done * (total - done) / 60))
return f"{line} · about {minutes} min left"
return line + _time_left(done, total, batches, elapsed)
def _file_size(path):
+115 -52
View File
@@ -20,14 +20,16 @@ keeping what it already wrote. Rows and log lines carry root-relative paths
and public root labels only.
"""
import asyncio
import functools
import os
from dataclasses import dataclass
import time
from dataclasses import dataclass, field
from aiohttp import web
from .. import model_gate
from .admin import _plural
from .admin import _plural, _time_left
from .annotation_suggest import (
OptionError,
unavailable_message,
@@ -35,6 +37,8 @@ from .annotation_suggest import (
)
from .annotations import (
_MESSAGES,
MAX_TARGET_PATH,
_coded,
_error,
_pinned_suggestions,
_settled,
@@ -71,9 +75,14 @@ except ImportError:
from utils.video_metadata import is_video_path
MAX_JOB_TARGETS = 500
# 500 targets of up to ~1.1 KiB each plus the options: the annotation routes'
# 64 KiB bound is for one target, too small for a full selection.
MAX_JOB_BODY_BYTES = 1024 * 1024
# 500 maximal targets plus the options: a path of MAX_TARGET_PATH code points
# takes at most 6 bytes each on the wire (a \uXXXX escape; raw UTF-8 is at most
# 4), and the root index and scope fit in 256. The annotation routes' 64 KiB
# bound is for one target, too small for a full selection.
MAX_JOB_BODY_BYTES = MAX_JOB_TARGETS * (MAX_TARGET_PATH * 6 + 256) + 64 * 1024
# A busy capture worker (a Tag dialog in another tab) is waited for this long,
# try by try, before the file is left for a rerun.
CAPACITY_BACKOFF = (0.1, 0.2, 0.4, 0.8)
ADD_TITLE = "Add tags to images"
AUTO_TITLE = "Auto-tag images"
@@ -97,12 +106,32 @@ SAVE_FAILED = "The tags could not be saved. Check server logs for details."
READ_FAILED = "The file could not be read."
SOURCE_CHANGED = "The source changed. Refresh the image collection."
STORAGE_RESTORED = "Prompt storage was restored; start the job again."
BUSY_RETRY = "The file could not be read now. Run the job again for it."
_monotonic = time.monotonic # the job's clock (a test can fake it)
class _JobStopped(Exception):
"""No later file can be written (prompt storage was restored)."""
@dataclass
class _Run:
"""One running job: its request, the gate owner and the resolved roots.
``roots`` maps each target's ``(root_index, root_scope)`` to ``(root,
label)``, read once per job off the event loop; ``root`` is None when the
scope no longer names a current root.
"""
job: object
spec: object
epoch: str
owner: object
service: object = None
roots: dict = field(default_factory=dict)
@dataclass(frozen=True)
class _Spec:
"""A validated start request."""
@@ -197,10 +226,26 @@ def _capture_outcome(exc: CaptureError, root, relative: str):
path = os.path.join(root, *relative.split("/"))
return "skipped", FILE_CHANGED if os.path.lexists(path) else FILE_NOT_FOUND
if isinstance(exc, CaptureCapacity):
return "errors", _MESSAGES["review-capacity"]
return "skipped", BUSY_RETRY
return "errors", READ_FAILED
async def _capacity_retried(job, attempt):
"""``await attempt()``, tried again while the capture store is at capacity.
Raises the last ``CaptureCapacity`` once ``CAPACITY_BACKOFF`` is spent or the
job was cancelled meanwhile.
"""
for delay in CAPACITY_BACKOFF:
try:
return await attempt()
except CaptureCapacity:
if job.cancelled:
raise
await asyncio.sleep(delay)
return await attempt()
def _load_model(service, model_type: str, use_gpu: bool) -> None:
"""Worker thread: load (or swap to) *model_type*; never downloads."""
if service.is_model_loaded() and service.get_loaded_model_type() == model_type:
@@ -225,10 +270,9 @@ class ImageTagJobsMixin:
spec, error = parse_image_tag_request(data)
if error is not None:
return error
for index, scope in dict.fromkeys((t[0], t[1]) for t in spec.targets):
_root, error = self._scoped_root(index, scope)
if error is not None:
return error
roots = await self._run_in_executor(self._job_roots, spec.targets)
if any(root is None for root, _label in roots.values()):
return _coded("source-changed")
epoch = await self._run_in_executor(self.db.get_execution_epoch)
# A running job in the way answers first: it says which job to wait for.
blocker = self._jobs.blocker(IMAGE_TAG_JOB_KIND, record_changing=True)
@@ -259,6 +303,18 @@ class ImageTagJobsMixin:
# -- runner ----------------------------------------------------------------
def _job_roots(self, targets) -> dict:
"""Worker thread: ``{(index, scope): (root or None, label)}`` (see _Run)."""
roots = list(self._get_all_output_dirs())
resolved = {}
for index, scope in dict.fromkeys((t[0], t[1]) for t in targets):
label = _public_path(roots[index]) if index < len(roots) else ""
error = self._gallery_scope_error(
{"root_scope": scope, "root": str(index)}, roots
)
resolved[(index, scope)] = (roots[index] if error is None else None, label)
return resolved
async def _run_image_tag_job(self, spec: _Spec, epoch: str, owner, job) -> None:
verb = _VERBS[spec.mode]
total = len(spec.targets)
@@ -267,14 +323,14 @@ class ImageTagJobsMixin:
if spec.mode == "auto":
started += f" · {spec.model_type}"
job.log_line(started)
run = _Run(job, spec, epoch, owner)
try:
service = None
if spec.mode == "auto":
service = await self._prepare_tagging_model(job, spec, owner)
if service is None:
run.service = await self._prepare_tagging_model(job, spec, owner)
if run.service is None:
job.finish("failed")
return
if not await self._tag_each_file(job, spec, epoch, owner, service):
if not await self._tag_each_file(run):
job.log_line(f"{verb} stopped · {STORAGE_RESTORED}")
job.finish("failed")
return
@@ -286,25 +342,27 @@ class ImageTagJobsMixin:
if owner is not None:
model_gate.release(owner)
async def _tag_each_file(self, job, spec, epoch, owner, service) -> bool:
async def _tag_each_file(self, run: _Run) -> bool:
"""Tag the targets in order; False when the job had to stop."""
total = len(spec.targets)
for number, target in enumerate(spec.targets, 1):
job, total = run.job, len(run.spec.targets)
run.roots = await self._run_in_executor(self._job_roots, run.spec.targets)
started = _monotonic()
for number, target in enumerate(run.spec.targets, 1):
if job.cancelled:
break
job.update(status_line=f"Processing file {number:,} of {total:,}")
# The scan's estimate (D56), with each finished file as one batch.
left = _time_left(number - 1, total, number - 1, _monotonic() - started)
job.update(status_line=f"Processing file {number:,} of {total:,}{left}")
job.publish({"type": "progress", "progress": job.progress})
stopped = False
try:
group, detail = await self._tag_one_file(
spec, epoch, owner, service, target
)
group, detail = await self._tag_one_file(run, target)
except _JobStopped:
stopped, group, detail = True, "errors", STORAGE_RESTORED
except Exception:
self.logger.exception("Image tagging failed for one file")
group, detail = "errors", SAVE_FAILED
self._record_tag_outcome(job, target, group, detail)
self._record_tag_outcome(run, target, group, detail)
job.update(processed=number, progress=number * 100 // total)
if stopped:
return False
@@ -337,57 +395,63 @@ class ImageTagJobsMixin:
return None
return service
async def _tag_one_file(self, spec, epoch, owner, service, target):
async def _verified(self, run: _Run, store, root, relative):
"""The current file's facts, waiting out a busy capture store."""
return await _capacity_retried(
run.job, lambda: self._run_in_executor(store.verify, root, relative)
)
async def _tag_one_file(self, run: _Run, target):
"""The (group, detail) of one target; raises _JobStopped on a restore."""
index, scope, relative = target
root, error = self._scoped_root(index, scope)
if error is not None:
root, _label = run.roots[(index, scope)]
if root is None:
return "errors", SOURCE_CHANGED
store = self._capture_store()
if spec.mode == "add":
if run.spec.mode == "add":
try:
facts = await self._run_in_executor(store.verify, root, relative)
facts = await self._verified(run, store, root, relative)
except CaptureError as exc:
return _capture_outcome(exc, root, relative)
return await self._union_image_tags(facts, list(spec.tags), epoch)
return await self._union_image_tags(facts, list(run.spec.tags), run.epoch)
if is_video_path(relative):
return "skipped", VIDEO_SKIPPED
try:
capture = await _settled(
_submit(
store.capture,
root,
relative,
epoch=epoch,
scope=scope,
root_index=index,
capture = await _capacity_retried(
run.job,
lambda: _settled(
_submit(
store.capture,
root,
relative,
epoch=run.epoch,
scope=scope,
root_index=index,
),
lambda made: made is not None and store.release(made.token),
),
lambda made: made is not None and store.release(made.token),
)
except CaptureError as exc:
return _capture_outcome(exc, root, relative)
try:
return await self._auto_tag_capture(
store, capture, spec, epoch, owner, service, root, relative
)
return await self._auto_tag_capture(run, store, capture, root, relative)
finally:
store.release(capture.token)
async def _auto_tag_capture(
self, store, capture, spec, epoch, owner, service, root, relative
):
async def _auto_tag_capture(self, run: _Run, store, capture, root, relative):
"""Suggest on the capture, then union only onto the same current bytes."""
if not capture.inferable:
return "skipped", NOT_A_STILL
if store.acquire(capture.token) is None:
raise _JobStopped # every capture was invalidated (restore)
spec = run.spec
try:
pairs = await model_gate.run(
owner,
run.owner,
_pinned_suggestions,
store,
capture.token,
service,
run.service,
capture,
spec.model_type,
spec.options,
@@ -397,7 +461,7 @@ class ImageTagJobsMixin:
self.logger.exception("Auto-tagging failed for one file")
return "errors", INFERENCE_FAILED
try:
facts = await self._run_in_executor(store.verify, root, relative)
facts = await self._verified(run, store, root, relative)
except CaptureError as exc:
return _capture_outcome(exc, root, relative)
if facts.locator != capture.locator or facts.sha256 != capture.sha256:
@@ -405,7 +469,7 @@ class ImageTagJobsMixin:
names = [str(name) for name, _score in pairs]
if not names:
return "unchanged", NO_NEW_TAGS
return await self._union_image_tags(facts, names, epoch)
return await self._union_image_tags(facts, names, run.epoch)
async def _union_image_tags(self, facts, tags: list, epoch: str):
"""``add_image_tags`` on the verified bytes, as a (group, detail)."""
@@ -434,12 +498,11 @@ class ImageTagJobsMixin:
return "tagged", f"Added {_plural(added, 'tag')}"
return "unchanged", NO_NEW_TAGS
def _record_tag_outcome(self, job, target, group: str, detail: str) -> None:
index, _scope, relative = target
roots = list(self._get_all_output_dirs())
label = _public_path(roots[index]) if index < len(roots) else ""
def _record_tag_outcome(self, run: _Run, target, group: str, detail: str) -> None:
index, scope, relative = target
_root, label = run.roots[(index, scope)]
where = f"{label}/{relative}" if label else relative
job.record(
run.job.record(
group,
{"path": relative, "root": label, "root_index": index, "detail": detail},
f"{_LOG_PREFIXES[group]} {where} · {detail}",
+3 -3
View File
@@ -861,11 +861,11 @@ test('an image tagging job shows Tagged Unchanged Skipped Errors and opens tagge
// An error, then the end: the header and the Activity item read "N tagged · M errors".
await step(request, job.id, { processed: 4, records: [
{ group: 'errors', path: 'broken/truncated.png', root: 'output', detail: 'The file could not be tagged.' },
{ group: 'errors', path: 'broken/truncated.png', root: 'output', detail: 'Auto-tagging failed. Check server logs for details.' },
] });
await expect(tiles(page).last().locator('dd')).toHaveText('1');
await finish(request, job.id);
await expect(toast(page)).toHaveText('Auto-tag images finished.');
await expect(toast(page)).toHaveText('Auto-tag images finished · 1 tagged · 1 error');
await expect(heading(page).locator('p.muted')).toHaveText(/^Finished \d\d:\d\d · 1 tagged · 1 error$/);
await expect(heading(page).locator('.heading-actions > button')).toHaveText(['Download report']);
await expect(progressSection(page).locator('.panel-heading > span.small')).toHaveText('4 of 4 files processed');
@@ -883,7 +883,7 @@ test('cancel tagging stops at a file boundary', async ({ page, request }) => {
await openJob(page, job.id);
await heading(page).getByRole('button', { name: 'Cancel tagging', exact: true }).focus();
await page.keyboard.press('Enter');
await expect(toast(page)).toHaveText('Add tags to images was cancelled.');
await expect(toast(page)).toHaveText('Add tags to images cancelled · 2 tagged');
await expect(heading(page).locator('h1')).toBeFocused();
await expect(heading(page).locator('p.muted')).toHaveText(/^Cancelled \d\d:\d\d at a file boundary · 2 tagged · completed work is kept$/);
await expect(heading(page).getByRole('button', { name: 'Cancel tagging', exact: true })).toHaveCount(0);
+57 -2
View File
@@ -84,7 +84,16 @@ test('Add tags runs a job and the cards show the tags after it finishes', async
const field = modal(page).getByRole('textbox', { name: 'Tags, separated by commas', exact: true });
await expect(field).toHaveValue('');
await expect(actionButtons(page)).toHaveText(['Cancel', 'Apply']);
let posted = 0;
page.on('request', item => { if (new URL(item.url()).pathname === '/prompt_manager/jobs/image-tags') posted += 1; });
// A field that parses to no names keeps the dialog and its text, and reports why on the field (D118).
await field.fill(' , ,');
await actionButtons(page).last().click();
await expect.poll(() => field.evaluate(node => node.validationMessage)).toBe('Enter at least one tag.');
await expect(title(page)).toHaveText('Tag selected images');
await expect(field).toHaveValue(' , ,');
await field.fill('sunset, Sunset , blue_hour,,');
expect(await field.evaluate(node => node.validity.valid)).toBe(true);
const started = jobStart(page);
const answer = page.waitForResponse(item => new URL(item.url()).pathname === '/prompt_manager/jobs/image-tags');
await actionButtons(page).last().click();
@@ -93,7 +102,10 @@ test('Add tags runs a job and the cards show the tags after it finishes', async
expect((await started).postDataJSON()).toEqual({ mode: 'add', tags: ['sunset', 'blue_hour'], targets: [
{ root_index: 0, root_scope: scope, path: FIRST }, { root_index: 0, root_scope: scope, path: SECOND }] });
const { job } = await (await answer).json();
// The refused Apply sent nothing: only the valid one was posted.
expect(posted).toBe(1);
await expect.poll(() => toasts(page)).toContain('Tagging started.');
expect(await toasts(page)).not.toContain('Enter at least one tag.');
await expect(modal(page)).toHaveCount(0);
await expect(bar(page).locator('strong')).toHaveText('2 selected');
expect((await ended(request, job.id)).counters).toEqual({ tagged: 2, unchanged: 0, skipped: 0, errors: 0 });
@@ -129,7 +141,6 @@ test('Auto-tag selected with Apply automatically runs a job with the chosen mode
await actionButtons(page).last().click();
expect(await limit.evaluate(node => node.validity.valid)).toBe(false);
await expect(title(page)).toHaveText('Auto-tag selected images');
expect(posted).toBe(0);
await limit.fill('2');
const started = jobStart(page);
const answer = page.waitForResponse(item => new URL(item.url()).pathname === '/prompt_manager/jobs/image-tags');
@@ -139,6 +150,8 @@ test('Auto-tag selected with Apply automatically runs a job with the chosen mode
options: { general_threshold: 0.5, character_threshold: 0.9, suggestion_limit: 2 },
targets: [{ root_index: 0, root_scope: scope, path: FIRST }, { root_index: 0, root_scope: scope, path: UNLINKED }] });
const { job } = await (await answer).json();
// The invalid press sent nothing: only this Start was posted.
expect(posted).toBe(1);
await expect.poll(() => toasts(page)).toContain('Auto-tagging started.');
await expect(modal(page)).toHaveCount(0);
await expect(bar(page).locator('strong')).toHaveText('2 selected');
@@ -226,7 +239,8 @@ test('Review each image walks the selection one dialog at a time and only approv
await actionButtons(page).last().click();
await expect(title(page)).toHaveText('Tag image · Image 2 of 3');
await expect(modal(page).locator('#tag-generation-status')).toHaveText('The tagging model is busy. Try again when it finishes.');
await expect(actionButtons(page)).toHaveText(['Retry', 'Stop reviewing', 'Skip', 'Approve and next']);
// No suggestions on screen: the primary would save the manual field only (D90).
await expect(actionButtons(page)).toHaveText(['Retry', 'Stop reviewing', 'Skip', 'Save and next']);
await actionButtons(page).nth(2).click();
await expect(title(page)).toHaveText('Tag image · Image 3 of 3');
await expect(chips(page)).toHaveCount(2);
@@ -245,6 +259,47 @@ test('Review each image walks the selection one dialog at a time and only approv
await expect.poll(async () => (await (await request.get('/__test__/tagging/captures')).json()).captures).toBe(0);
});
test('Review holds Stop reviewing, Escape and × while a save is in flight, and names a manual-only save', async ({ page, request }) => {
await select(page, FIRST, SECOND);
let release;
const held = new Promise(resolve => { release = resolve; });
await page.route('**/prompt_manager/image-annotations/tags', async route => {
if (route.request().method() === 'PUT') await held;
await route.continue();
});
await barButton(page, 'Auto-tag selected').click();
await modal(page).getByRole('radio', { name: 'Review each image', exact: true }).check();
await actionButtons(page).last().click();
await expect(title(page)).toHaveText('Tag image · Image 1 of 2');
await expect(chips(page)).not.toHaveCount(0);
await expect(actionButtons(page)).toHaveText(['Stop reviewing', 'Skip', 'Approve and next']);
// D90: a model change clears the suggestions, so the primary saves the manual field only and says so.
await modal(page).getByRole('combobox', { name: 'Tagging model', exact: true }).selectOption('wd14-vit');
await expect(chips(page)).toHaveCount(0);
await expect(actionButtons(page)).toHaveText(['Stop reviewing', 'Skip', 'Save and next']);
await modal(page).getByRole('textbox', { name: 'Image tags · edit manually', exact: true }).fill('mine');
const put = page.waitForRequest(item => item.method() === 'PUT' && item.url().endsWith('/image-annotations/tags'));
await actionButtons(page).last().click();
await put;
const stop = actionButtons(page).first();
await expect(stop).toHaveAttribute('aria-disabled', 'true');
await expect(actionButtons(page).nth(1)).toHaveAttribute('aria-disabled', 'true');
// None of these ends the session while the save is in flight; the count then includes it.
await page.keyboard.press('Escape');
await modal(page).locator('#modal-close').click();
await stop.dispatchEvent('click'); // aria-disabled: Playwright's click would wait for it
release();
await expect(title(page)).toHaveText('Tag image · Image 2 of 2');
await expect(chips(page)).not.toHaveCount(0);
await expect(actionButtons(page)).toHaveText(['Stop reviewing', 'Skip', 'Approve and finish']);
await expect(actionButtons(page).first()).not.toHaveAttribute('aria-disabled', 'true');
await actionButtons(page).first().click();
await expect(modal(page)).toHaveCount(0);
await expect(toast(page)).toHaveText('1 of 2 images tagged.');
expect(await savedTags(request, FIRST)).toEqual(['mine']);
expect(await savedTags(request, SECOND)).toEqual([]);
});
test('a running scan makes the start report busy', async ({ page, request }) => {
const scan = await (await request.post('/__test__/jobs/start', { data: { kind: 'scan', title: 'Scan for metadata' } })).json();
try {
@@ -300,6 +300,13 @@ test('image-tags jobs get their own tiles outcomes and pill verb', async () => {
assert.equal(outcomeLine(job), '1,420 tagged · 1 error');
assert.equal(outcomeLine({ ...job, counters: {} }), '0 tagged · 0 errors');
assert.equal(startToast(job), 'Auto-tagging started.');
// The design's toast pattern, as the scan's: "<Job> finished · N tagged · M errors".
const { finishToast } = await source;
assert.equal(finishToast({ ...job, state: 'finished' }), 'Auto-tag images finished · 1,420 tagged · 1 error');
assert.equal(finishToast({ ...job, state: 'cancelled' }), 'Auto-tag images cancelled · 1,420 tagged');
assert.equal(finishToast({ ...job, state: 'failed' }), 'Auto-tag images failed · 1,420 tagged · 1 error');
assert.equal(finishToast({ ...job, title: 'Add tags to images', counters: {}, state: 'finished' }),
'Add tags to images finished · 0 tagged · 0 errors');
});
test('unknown kinds keep the generic view', async () => {
@@ -49,6 +49,24 @@ test('add parses comma tags with the same dedupe as the dialog', () => {
assert.equal(parseBulkTags, state.parseManualTags);
});
test('Add tags keeps its dialog and the typed text when the field parses to no names', async () => {
const { openAddTags, NO_TAGS } = module();
const toasts = [];
const posted = [];
const store = { start: async (kind, body) => { posted.push(body); return { job: { id: JOB_ID, state: 'running' } }; },
find: () => null, subscribe: () => () => {} };
let dialog = null;
openAddTags({ scope: pageScope(), store, images: [image('a.png')], trigger: null, notify: text => toasts.push(text),
openInput: args => { dialog = args; } });
assert.equal(typeof dialog?.validate, 'function');
// The dialog reports the message on its field and stays open: nothing toasts, nothing is sent.
assert.equal(dialog.validate(' , ,'), NO_TAGS);
assert.equal(dialog.validate('sunset'), null);
assert.deepEqual(toasts, []);
await dialog.onApply('sunset, Sunset');
assert.deepEqual(posted.map(body => body.tags), [['sunset']]);
});
test('busy start keeps the selection and names the running job', async () => {
const { startTagJob } = module();
const toasts = [];
+139
View File
@@ -32,6 +32,7 @@ from database.execution_history_schema import rotate_history_epoch
from py import model_gate
from py.api import jobs as jobs_module
from py.config import AutotagConfig
from utils.media_capture import CaptureCapacity
START = "/prompt_manager/jobs/image-tags"
WAIT = 30.0
@@ -644,5 +645,143 @@ class TestStartAndExclusion(ImageTagJobTestCase):
self.assertTrue(any("output/sub/a.png" in line for line in detail["log"]))
class TestFixRound(ImageTagJobTestCase):
"""The final review's fixes: estimate, body bound, busy captures, roots."""
async def test_status_line_estimates_minutes_left_after_the_minimum(self):
for index in range(3):
self.png(f"f{index}.png", color=(index, 7, 7))
clock = iter([0.0, 0.0, 120.0, 240.0])
lines = []
self.tagger.on_generate = lambda _path: lines.append(
self.api._jobs.list()[0].status_line
)
with patch(
"py.api.image_tag_jobs._monotonic", side_effect=lambda: next(clock, 240.0)
):
await self.run_job(self.auto_body([f"f{i}.png" for i in range(3)]))
# Two files make the minimum (the scan's MIN_BATCHES_FOR_ESTIMATE); then
# 120 s per file leaves about 2 min for the third.
self.assertEqual(
lines,
[
"Processing file 1 of 3",
"Processing file 2 of 3",
"Processing file 3 of 3 · about 2 min left",
],
)
def maximal_target(self, number):
"""A distinct target whose path is 1024 non-ASCII code points long."""
head = f"{number:03d}/" + ("界" * 200 + "/") * 5
path = head + "界" * (1024 - len(head) - 4) + ".png"
self.assertEqual(len(path), 1024)
return self.target(path)
async def test_start_body_bounds(self):
from py.api import image_tag_jobs
body = {
"mode": "add",
"targets": [self.maximal_target(n) for n in range(500)],
"tags": ["x"],
}
# Escaped as \uXXXX (6 bytes a code point), far over the old 1 MiB.
payload = json.dumps(body).encode("utf-8")
self.assertGreater(len(payload), 3 * 1000 * 1000)
response = await self.client.post(
START, data=payload, headers={"Content-Type": "application/json"}
)
data = await self.json_of(response)
self.assertEqual(response.status, 202, data)
self.assertEqual(data["job"]["scope"]["images"], 500)
job = self.api._jobs.get(data["job"]["id"])
await self.client.post(f"/prompt_manager/jobs/{job.id}/cancel")
await asyncio.wait_for(asyncio.shield(job.task), WAIT)
cap = image_tag_jobs.MAX_JOB_BODY_BYTES
self.assertGreaterEqual(cap, len(payload))
over = b'{"mode":"add","targets":[],"tags":["x"]'
over += b" " * (cap + 1 - len(over) - 1) + b"}"
self.assertEqual(len(over), cap + 1)
response = await self.client.post(
START, data=over, headers={"Content-Type": "application/json"}
)
self.assertEqual(response.status, 413)
def saturate(self, store):
with store._lock:
store._workers += store.max_workers
self.addCleanup(self.unsaturate, store)
def unsaturate(self, store):
with store._lock:
if store._workers >= store.max_workers:
store._workers -= store.max_workers
async def test_busy_capture_workers_are_retried_then_left_for_a_rerun(self):
self.png("a.png")
self.png("b.png", color=(1, 2, 3))
store = self.api._capture_store()
self.saturate(store)
with patch("py.api.image_tag_jobs.CAPACITY_BACKOFF", (0.001, 0.001)):
added = await self.run_job(self.add_body(["a.png"], ["x"]))
auto = await self.run_job(self.auto_body(["b.png"]))
retry = "The file could not be read now. Run the job again for it."
for job, path in ((added, "a.png"), (auto, "b.png")):
self.assertEqual(job.state, "finished")
self.assertEqual(self.rows(job, "skipped"), [(path, retry)])
self.assertEqual(job.counters["errors"], 0)
self.assertNotIn("Tag dialog", json.dumps(job.detail()))
self.assertEqual(self.tagger.calls, [])
self.assertEqual(self.stored("a.png")["tags"], [])
async def test_a_busy_capture_worker_that_frees_up_is_tagged(self):
self.png("a.png")
store = self.api._capture_store()
real = store.verify
tries = []
def verify(root, relative):
tries.append(relative)
if len(tries) < 3:
raise CaptureCapacity("Too many captures are in progress")
return real(root, relative)
with (
patch.object(store, "verify", side_effect=verify),
patch("py.api.image_tag_jobs.CAPACITY_BACKOFF", (0.001, 0.001, 0.001)),
):
job = await self.run_job(self.add_body(["a.png"], ["x"]))
self.assertEqual(len(tries), 3)
self.assertEqual(self.rows(job, "tagged"), [("a.png", "Added 1 tag")])
self.assertEqual(self.stored("a.png")["tags"], ["x"])
async def test_output_roots_are_read_once_per_job_off_the_event_loop(self):
for index in range(3):
self.png(f"f{index}.png", color=(index, 1, 1))
body = self.add_body([f"f{i}.png" for i in range(3)], ["x"])
original = self.api._get_all_output_dirs
threads = []
def lookup():
threads.append(threading.current_thread())
return original()
with patch.object(self.api, "_get_all_output_dirs", side_effect=lookup):
job = await self.run_job(body)
self.assertEqual(job.counters["tagged"], 3)
# One read when the start checks the scopes, one for the whole run.
self.assertEqual(len(threads), 2)
self.assertNotIn(threading.main_thread(), threads)
self.assertEqual(job.outcomes["tagged"][0]["root"], "output")
if __name__ == "__main__":
unittest.main()
+4 -4
View File
@@ -169,7 +169,7 @@
},
"workspace/activity-store.mjs": {
"lines": 98,
"branches": 95,
"branches": 96,
"functions": 98,
"statements": 98
},
@@ -230,7 +230,7 @@
"workspace/image-tagging.mjs": {
"lines": 97,
"branches": 90,
"functions": 87,
"functions": 88,
"statements": 97
},
"workspace/images-category.mjs": {
@@ -265,8 +265,8 @@
},
"workspace/images-tagging-bulk.mjs": {
"lines": 98,
"branches": 91,
"functions": 85,
"branches": 93,
"functions": 86,
"statements": 98
},
"workspace/images-tagging-review.mjs": {
+8 -1
View File
@@ -112,13 +112,20 @@ const errorsOf = job => `${number(job.counters.errors ?? 0)} ${noun(job.counters
/** "1,420 added". */
export const addedOf = job => `${number(job.counters.added ?? 0)} added`;
/** The toast when a job ends: "Scan finished · N added · M errors", "Scan cancelled · N added". */
/**
* The toast when a job ends: "Scan finished · N added · M errors", "Scan cancelled · N added"; an image tagging job
* the same way with its own title and count (D89): "Auto-tag images finished · N tagged · M errors".
*/
export function finishToast(job) {
if (job.kind === 'scan') {
if (job.state === 'cancelled') return `Scan cancelled · ${addedOf(job)}`;
if (job.state === 'failed') return `Scan failed · ${addedOf(job)} · ${errorsOf(job)}`;
return `Scan finished · ${addedOf(job)} · ${errorsOf(job)}`;
}
if (job.kind === 'image-tags') {
if (job.state === 'cancelled') return `${job.title} cancelled · ${taggedOf(job)}`;
return `${job.title} ${job.state === 'failed' ? 'failed' : 'finished'} · ${outcomeLine(job)}`;
}
if (job.state === 'cancelled') return `${job.title} was cancelled.`;
if (job.state === 'failed') return `${job.title} failed.`;
return `${job.title} finished.`;
+9 -5
View File
@@ -275,7 +275,9 @@ export function openImageTagging({api, scope, image, source = 'images', trigger
const skip = createButton('Skip');
const primary = createButton('Save manual tags', 'ca-primary');
const last = review && position.index === position.total;
const approveLabel = last ? 'Approve and finish' : 'Approve and next';
// D90: Approve with suggestions on screen; after a model change cleared them (M2) Save keeps only the manual field.
const reviewLabel = state => (state.generated ? (last ? 'Approve and finish' : 'Approve and next')
: (last ? 'Save and finish' : 'Save and next'));
if (review) actions.append(cancel, skip, primary);
else actions.append(cancel, primary);
@@ -343,9 +345,10 @@ export function openImageTagging({api, scope, image, source = 'images', trigger
if (state.suggestions !== shownRows || state.suggestionsText !== shownText) renderSuggestions(state);
setText(count, state.countText);
setText(status, state.statusText);
setText(primary, review ? approveLabel : state.primaryLabel);
setText(primary, review ? reviewLabel(state) : state.primaryLabel);
setDisabled(primary, !state.canSave);
if (review) setDisabled(skip, state.phase === 'saving');
// D90: while Approve's save is in flight the session cannot be left, so its count includes that save.
if (review) for (const button of [skip, cancel]) setDisabled(button, state.phase === 'saving');
if (state.canRetry !== retry.isConnected) {
if (state.canRetry) actions.prepend(retry); // D116
else {
@@ -356,7 +359,7 @@ export function openImageTagging({api, scope, image, source = 'images', trigger
if (state.phase === 'saved') complete(state);
}
cancel.addEventListener('click', () => closeWorkspaceDialog());
cancel.addEventListener('click', () => { if (!disabled(cancel)) closeWorkspaceDialog(); });
skip.addEventListener('click', () => {
if (!open || advancing || disabled(skip)) return;
advancing = true;
@@ -382,7 +385,8 @@ export function openImageTagging({api, scope, image, source = 'images', trigger
if (review && !advancing) onStop?.();
}
const dialogTitle = review ? `Tag image · Image ${position.index} of ${position.total}` : 'Tag image'; // D90
openContentDialog({scope, title: dialogTitle, nodes, trigger, onClose: release});
openContentDialog({scope, title: dialogTitle, nodes, trigger, onClose: release,
canClose: review ? () => controller.state.phase !== 'saving' : null});
document.getElementById('modal')?.classList.add('tagging-modal');
local.cleanup(controller.subscribe(render));
render(controller.state);
+11 -7
View File
@@ -26,14 +26,14 @@ import { showWorkspaceToast } from './toast.mjs';
export const IMAGE_TAG_JOB = 'image-tags';
export const MAX_BULK_IMAGES = 500;
export const TOO_MANY = 'Select at most 500 images.';
export const NO_TAGS = 'Enter at least one tag.'; // D117
export const NO_TAGS = 'Enter at least one tag.'; // D118
export const ADD_DIALOG = Object.freeze({title: 'Tag selected images', label: 'Tags, separated by commas', action: 'Apply'});
export const AUTO_TITLE = 'Auto-tag selected images';
export const APPLY_CHOICES = Object.freeze([
Object.freeze({value: 'auto', label: 'Apply automatically', action: 'Start auto-tagging'}),
Object.freeze({value: 'review', label: 'Review each image', action: 'Start review'}),
]);
// D117: the concept has no copy for these; they follow D55's "The scan could not start. Try again.".
// D118: the concept has no copy for these; they follow D55's "The scan could not start. Try again.".
const START_FAILED = Object.freeze({add: 'Tagging could not start. Try again.', auto: 'Auto-tagging could not start. Try again.'});
const FINAL = new Set(['finished', 'cancelled', 'failed']);
@@ -102,13 +102,17 @@ function followTagged({store, scope, job, images, onTagged}) {
followJobEnd({store, scope, id: job.id, onEnd: ended => { if ((ended.counters.tagged ?? 0) > 0) onTagged(keys); }});
}
/** Add tags: the concept's simpleInput, then the job over `images` (the checked files, taken now). */
export function openAddTags({scope, store, images, trigger, onTagged = () => {}, notify = showWorkspaceToast}) {
/**
* Add tags: the concept's simpleInput, then the job over `images` (the checked files, taken now). A field that parses
* to no names keeps the dialog and its text, and reports NO_TAGS on the field (D118). `openInput` is the dialog seam.
*/
export function openAddTags({scope, store, images, trigger, onTagged = () => {}, notify = showWorkspaceToast,
openInput = openInputDialog}) {
const targets = bulkTargets(images);
if (!targets) { notify(TOO_MANY); return; }
openInputDialog({scope, trigger, ...ADD_DIALOG, onApply: async text => {
openInput({scope, trigger, ...ADD_DIALOG, validate: text => (parseBulkTags(text).length ? null : NO_TAGS),
onApply: async text => {
const tags = parseBulkTags(text);
if (!tags.length) { notify(NO_TAGS); return; }
const job = await startTagJob({store, scope, body: {mode: 'add', targets, tags}, notify});
if (job) followTagged({store, scope, job, images, onTagged});
}});
@@ -145,7 +149,7 @@ export function openAutoTag({api, scope, store, images, trigger, onTagged = () =
const options = createTaggingOptionsController({api});
// D85: the Tag image dialog's model panel without Generate; its Suggestions field is the job's count (D91).
const {element: panel, validate} = renderModelPanel({scope: local, controller: options,
heading: `Auto-tag ${countOf(listed.length, 'image')}`, generate: false}); // D117
heading: `Auto-tag ${countOf(listed.length, 'image')}`, generate: false}); // D118
let choice = APPLY_CHOICES[0];
const actions = el('div', undefined, 'dialog-actions');
const cancel = createButton('Cancel');
+28 -8
View File
@@ -59,10 +59,10 @@ function createShell() {
body.id = 'modal-body';
header.append(title, close);
dialog.append(header, body);
close.addEventListener('click', () => finish());
close.addEventListener('click', () => dismiss());
dialog.addEventListener('cancel', event => {
event.preventDefault();
finish();
dismiss();
});
// A browser may still close a modal on its own; that is a dismissal too. `close` arrives as a later task, so it
// is ignored once this shell was reopened or replaced.
@@ -82,6 +82,12 @@ function finish() {
if (trigger?.isConnected) trigger.focus();
}
/** × or Escape: close, unless the open dialog's `canClose()` holds it (a save in flight). */
function dismiss() {
if (session?.canClose && !session.canClose()) return;
finish();
}
const boundScopes = new WeakSet();
function requireScope(scope) {
@@ -104,13 +110,13 @@ function bindScope(scope) {
});
}
function show(heading, nodes, { scope, trigger, focusTarget, onClose = null }) {
function show(heading, nodes, { scope, trigger, focusTarget, onClose = null, canClose = null }) {
shell ??= createShell();
if (!shell.dialog.isConnected) document.body.append(shell.dialog);
bindScope(scope);
// A swapped-out body is gone from the shell, so its owner releases it.
const replaced = session?.onClose;
session = { scope, trigger: trigger ?? session?.trigger ?? null, onClose };
session = { scope, trigger: trigger ?? session?.trigger ?? null, onClose, canClose };
replaced?.();
shell.title.textContent = heading;
shell.body.replaceChildren(...nodes);
@@ -298,10 +304,14 @@ export function openCopyFallback(text, { scope, trigger = null } = {}) {
* The concept's `simpleInput`: one labelled `input#action-input` (starting at `value`), then Cancel and the primary
* `action`. The action reports the trimmed value and closes; an empty value keeps the dialog open on its field, or
* reports `fallback` when one is given (Save view's "Saved view", a1339). A positive `maxLength` limits the field.
* `validate(value)` may refuse a value with a message: the dialog then stays open with the typed text and the field
* reports the message (its native validity) until it is edited.
*/
export function openInputDialog({ scope, title, label, action, value = '', maxLength, fallback, onApply, trigger = null }) {
export function openInputDialog({ scope, title, label, action, value = '', maxLength, fallback, onApply, trigger = null,
validate = null }) {
requireScope(scope);
requireCallbacks({ onApply });
if (validate !== null) requireCallbacks({ validate });
for (const [name, text] of Object.entries({ title, label, action })) {
if (typeof text !== 'string' || !text) throw new TypeError(`The input dialog needs a ${name}`);
}
@@ -314,9 +324,17 @@ export function openInputDialog({ scope, title, label, action, value = '', maxLe
const cancel = createButton('Cancel');
const apply = createButton(action, 'ca-primary');
cancel.addEventListener('click', () => finish());
input.addEventListener('input', () => input.setCustomValidity(''));
apply.addEventListener('click', () => {
const entered = input.value.trim() || fallback;
if (!entered) { input.focus(); return; }
const problem = validate?.(entered);
if (problem) {
input.setCustomValidity(problem);
input.reportValidity();
input.focus();
return;
}
finish();
onApply(entered);
});
@@ -327,14 +345,16 @@ export function openInputDialog({ scope, title, label, action, value = '', maxLe
/**
* The concept's plain `dialog(title, body)` with page-built `nodes`. `onClose` runs once when the dialog closes or
* another dialog replaces its body, so the page can dispose what it mounted there.
* another dialog replaces its body, so the page can dispose what it mounted there. While `canClose()` answers false,
* × and Escape leave the dialog open (the page, a route change or a replacing dialog still close it).
*/
export function openContentDialog({ scope, title, nodes, onClose = null, trigger = null }) {
export function openContentDialog({ scope, title, nodes, onClose = null, trigger = null, canClose = null }) {
requireScope(scope);
if (typeof title !== 'string' || !title) throw new TypeError('The dialog needs a title');
if (!Array.isArray(nodes) || nodes.some(node => !node || typeof node !== 'object')) throw new TypeError('The dialog needs its nodes');
if (onClose !== null && typeof onClose !== 'function') throw new TypeError('The dialog onClose must be a function');
show(title, nodes, { scope, trigger, onClose });
if (canClose !== null && typeof canClose !== 'function') throw new TypeError('The dialog canClose must be a function');
show(title, nodes, { scope, trigger, onClose, canClose });
}
export const SCAN_REVIEW_SENTENCE = 'Enrich positive / negative associations, reuse canonical text and report unchanged files. ' +