fix(tagging): keep tags typed before the image observation answers (#515)

The Tag image field is editable while the dialog observes the file, but the
observation's answer overwrote it with the saved set, so typing then Save
wrote the old tags ("0 tags saved"). The controller now reads the draft when
the answer arrives: text typed during a first observation (or before Retry
of a failed one) follows the saved tags, and a kept draft takes the text
typed while the reopen observes.

tests/browser/malloc_tuning.py

@@ -1,31 +0,0 @@
-"""Keep the browser test server's large frees going back to the OS (#516).
-
-glibc serves an allocation of at least the mmap threshold (128 KiB at start) with its
-own mmap and unmaps it on free. Freeing such a block also raises the threshold, up to
-32 MiB, so the server's later image-sized buffers (PIL pixel data, PNG encoding,
-whole-file reads in the executor threads) come from per-thread malloc arenas instead.
-Those arenas fragment and keep their pages: a server grew from 13 MB to 0.6-1 GB RSS
-over one page-workflow shard while its Python heap stayed near 12 MB. Setting
-M_MMAP_THRESHOLD fixes the threshold at its start value and turns the raising off.
-"""
-
-import ctypes
-import sys
-
-M_MMAP_THRESHOLD = -3
-MMAP_THRESHOLD_BYTES = 128 * 1024
-
-
-def pin_mmap_threshold(libc=None) -> bool:
-    """Fix glibc's mmap threshold; False where there is no glibc ``mallopt``."""
-    if not sys.platform.startswith("linux"):
-        return False
-    if libc is None:
-        try:
-            libc = ctypes.CDLL(None)
-        except OSError:
-            return False
-    mallopt = getattr(libc, "mallopt", None)
-    if mallopt is None:
-        return False
-    return bool(mallopt(M_MMAP_THRESHOLD, MMAP_THRESHOLD_BYTES))

tests/browser/server.py

@@ -20,7 +20,6 @@ from PIL import Image
 from PIL.PngImagePlugin import PngInfo
 
 from tests.browser.folder_trash import close_trash, replace_folder
-from tests.browser.malloc_tuning import pin_mmap_threshold
 from tests.browser.request_hold import register_request_holds
 from tests.browser import slow_reset_watch
 from tests.browser.fast_reset import SEED_ON_LIVE_ENV, ScratchSeeder
@@ -756,8 +755,6 @@ def main(argv: list[str] | None = None) -> None:
         help="shut down when stdin closes (the launcher's parent-death guard)",
     )
     args = parser.parse_args(argv)
-    # Before any thread allocates (#516): large frees then return to the OS.
-    pin_mmap_threshold()
     listener = bind_listener(args.port)
     # One machine-readable line tells the launcher which port this process holds.
     print(f"{PORT_ANNOUNCEMENT}{listener.getsockname()[1]}", flush=True)

tests/browser/workspace-review.spec.mjs

@@ -221,6 +221,33 @@ test('tag image from review saves tags and the item remains pending', async ({ p
     await expect(dialog).toHaveCount(0);
 });
 
+test('#515: tags typed before the image observation answers are saved', async ({ page, request }) => {
+    await seed(request, 1);
+    await open(page);
+    // Hold the dialog's observation so the typing lands while it is pending (a slow host or WebKit in CI).
+    let release;
+    const held = new Promise(resolve => { release = resolve; });
+    const matcher = '**/prompt_manager/image-annotations/observe';
+    let reachedObserve;
+    const reached = new Promise(resolve => { reachedObserve = resolve; });
+    await page.route(matcher, async route => { reachedObserve(); await held; await route.continue(); });
+    await panel(page).getByRole('button', { name: 'Tag image', exact: true }).click();
+    const dialog = page.getByRole('dialog', { name: 'Tag image', exact: true });
+    await reached;
+    const field = dialog.getByRole('textbox', { name: 'Image tags · edit manually', exact: true });
+    await field.fill('dunes');
+    const save = dialog.getByRole('button', { name: 'Save manual tags', exact: true });
+    await expect(save).toHaveAttribute('aria-disabled', 'true');
+    const answered = page.waitForResponse(item => item.url().endsWith('/image-annotations/observe'));
+    release();
+    await answered;
+    await expect(save).not.toHaveAttribute('aria-disabled', 'true');
+    await expect(field).toHaveValue('dunes');
+    await save.click();
+    await expect(toast(page)).toHaveText('1 tags saved for review_0000.png. Prompt association still awaits review.');
+    await page.unroute(matcher);
+});
+
 test('an empty queue shows the empty notice only', async ({ page, request }) => {
     await seed(request, 0);
     await page.goto(review);

tests/js/workspace-image-tagging-state.test.js

@@ -876,3 +876,63 @@ test('retry after a changed storage, file or expired capture observes again and
     await settle();
     assert.equal(c.state.manual, 'Minimal, warm_light');
 });
+
+test('#515: text typed while the first observation is pending is kept, after the saved tags', async () => {
+    const { createImageTaggingController } = await load();
+    const answers = [deferred(), deferred()];
+    let n = 0;
+    const h = harness(defaultRoutes({ [`POST ${OBSERVE}`]: () => answers[n++].promise }));
+    // An untagged file: the typed text is the whole draft and Save sends it.
+    const bare = createImageTaggingController({ api: h.api, target: TARGET, source: 'review' });
+    const first = bare.open();
+    bare.setManual('dunes');
+    answers[0].resolve(response(200, observed({ tags: [], tag_revision: 0 })));
+    await first;
+    await settle();
+    assert.equal(bare.state.phase, 'ready');
+    assert.equal(bare.state.manual, 'dunes');
+    assert.equal(bare.state.statusText, '', 'nothing changed elsewhere');
+    await bare.save();
+    await settle();
+    assert.deepEqual(h.of('PUT', TAGS).at(-1).body.tags, ['dunes']);
+    // A tagged file: its saved tags lead, so a Save never drops tags the user had not seen yet.
+    const tagged = createImageTaggingController({ api: h.api, target: TARGET });
+    const second = tagged.open();
+    tagged.setManual('dunes, minimal');
+    answers[1].resolve(response(200, observed()));
+    await second;
+    await settle();
+    assert.equal(tagged.state.manual, 'Minimal, warm_light, dunes');
+    assert.equal(tagged.state.statusText, '');
+});
+
+test('#515: a kept draft takes the text typed while the reopen observes', async () => {
+    const { c } = await opened({ [`PUT ${TAGS}`]: () => fail(409, 'file-changed', 'The file changed.') });
+    c.setManual('kept draft');
+    await c.save();
+    await settle();
+    const reopened = c.retry();
+    c.setManual('kept draft, later');
+    await reopened;
+    await settle();
+    assert.equal(c.state.phase, 'ready');
+    assert.equal(c.state.manual, 'kept draft, later');
+});
+
+test('#515: text typed before Retry after a failed first observation follows the saved tags', async () => {
+    const { createImageTaggingController } = await load();
+    let n = 0;
+    const h = harness(defaultRoutes({
+        [`POST ${OBSERVE}`]: () => (n++ === 0 ? fail(500, 'internal', 'boom') : response(200, observed())),
+    }));
+    const c = createImageTaggingController({ api: h.api, target: TARGET });
+    await c.open();
+    await settle();
+    assert.equal(c.state.phase, 'observe-error');
+    c.setManual('dunes');
+    await c.retry();
+    await settle();
+    assert.equal(c.state.phase, 'ready');
+    assert.equal(c.state.manual, 'Minimal, warm_light, dunes');
+    assert.equal(c.state.statusText, '');
+});

tests/test_browser_malloc_tuning.py

@@ -1,92 +0,0 @@
-"""#516: the browser test server pins glibc's mmap threshold.
-
-Large frees then go back to the OS instead of fragmenting per-thread arenas.
-"""
-
-import ctypes
-import sys
-import unittest
-from unittest import mock
-
-from tests.browser import malloc_tuning
-
-
-class FakeLibc:
-    def __init__(self, result=1):
-        self.calls = []
-        self.result = result
-
-    def mallopt(self, option, value):
-        self.calls.append((option, value))
-        return self.result
-
-
-class PinMmapThresholdTests(unittest.TestCase):
-    def test_sets_a_fixed_threshold_through_mallopt(self):
-        libc = FakeLibc()
-        with mock.patch.object(sys, "platform", "linux"):
-            self.assertTrue(malloc_tuning.pin_mmap_threshold(libc))
-        # M_MMAP_THRESHOLD is -3; setting it also turns off glibc's dynamic
-        # raising of the threshold.
-        self.assertEqual(libc.calls, [(-3, 128 * 1024)])
-
-    def test_reports_a_refused_setting(self):
-        with mock.patch.object(sys, "platform", "linux"):
-            self.assertFalse(malloc_tuning.pin_mmap_threshold(FakeLibc(result=0)))
-
-    def test_skips_a_libc_without_mallopt(self):
-        with mock.patch.object(sys, "platform", "linux"):
-            self.assertFalse(malloc_tuning.pin_mmap_threshold(object()))
-
-    def test_skips_other_platforms_without_loading_a_libc(self):
-        for name in ("win32", "darwin"):
-            with (
-                self.subTest(name),
-                mock.patch.object(sys, "platform", name),
-                mock.patch.object(
-                    ctypes, "CDLL", side_effect=AssertionError("no libc load")
-                ),
-            ):
-                self.assertFalse(malloc_tuning.pin_mmap_threshold())
-
-    def test_skips_when_the_process_libc_cannot_be_loaded(self):
-        with (
-            mock.patch.object(sys, "platform", "linux"),
-            mock.patch.object(ctypes, "CDLL", side_effect=OSError("no libc")),
-        ):
-            self.assertFalse(malloc_tuning.pin_mmap_threshold())
-
-    @unittest.skipUnless(
-        sys.platform.startswith("linux")
-        and hasattr(ctypes.CDLL(None), "gnu_get_libc_version"),
-        "glibc only",
-    )
-    def test_glibc_accepts_the_setting(self):
-        # This pins the threshold for the rest of this test process too; the
-        # test server does the same at start, so nothing here depends on it.
-        self.assertTrue(malloc_tuning.pin_mmap_threshold())
-
-
-class ServerMainTests(unittest.TestCase):
-    def test_main_pins_the_threshold_before_it_starts_serving(self):
-        from tests.browser import server
-
-        order = []
-
-        def bind(port):
-            order.append("bind")
-            raise RuntimeError("stop before serving")
-
-        with (
-            mock.patch.object(
-                server, "pin_mmap_threshold", side_effect=lambda: order.append("pin")
-            ),
-            mock.patch.object(server, "bind_listener", side_effect=bind),
-        ):
-            with self.assertRaises(RuntimeError):
-                server.main(["--port", "0"])
-        self.assertEqual(order, ["pin", "bind"])
-
-
-if __name__ == "__main__":
-    unittest.main()

tests/test_bulk_library_delete.py

@@ -64,15 +64,14 @@ def table_dump(db):
 class LibraryDeleteCase(unittest.TestCase):
     def setUp(self):
         self.temp = tempfile.TemporaryDirectory()
-        # Cleanups run last-in first-out and after every cleanup a test adds,
-        # so a database a test opens (and closes via addCleanup) is closed
-        # before the folder goes; Windows cannot delete an open file (#510).
-        self.addCleanup(self.temp.cleanup)
         self.root = Path(self.temp.name)
         self.db = PromptDatabase(str(self.root / "delete.db"))
-        self.addCleanup(self.db.close_all)
         self.epoch = self.db.get_library_options()["library_epoch"]
 
+    def tearDown(self):
+        self.db.close_all()
+        self.temp.cleanup()
+
     def paired(self, positive, negative="blur"):
         pair = self.db.save_prompt_pair(positive, negative)
         found = self.db.get_prompt_pair(pair)

web/js/workspace/image-tagging-state.mjs

@@ -317,6 +317,9 @@ export function createImageTaggingController({api, target, source = 'images', on
     // A save in flight: closing hands the capture release to its completion.
     let saving = false;
     let observedOnce = false;
+    // #515: the user typed since the last observation started (or before the first one answered), so the answer
+    // keeps that text.
+    let typed = false;
 
     function set(patch) {
         if (closed) return;
@@ -345,7 +348,8 @@ export function createImageTaggingController({api, target, source = 'images', on
     async function open({keepDraft = false} = {}) {
         if (closed) return;
         const serial = ++openSerial;
-        const draft = keepDraft ? state.manual : null;
+        // A reopen after an observation starts from its answer; text typed before any observation is kept (#515).
+        if (observedOnce) typed = false;
         const seenRevision = state.tagRevision;
         stopGenerating();
         releaseCapture();
@@ -385,15 +389,19 @@ export function createImageTaggingController({api, target, source = 'images', on
         const savedModel = !chosen && !modelPicked && read.model ? {model: read.model} : {};
         observedOnce = true;
         const {width, height, media_type: mediaType, preview_url: previewUrl, generate} = value.capture;
-        // A kept draft stays as typed unless another writer changed the set meanwhile: then the set leads it (I2).
-        const changed = draft !== null && value.tag_revision !== seenRevision;
+        // The draft is read now, so text typed while observing is kept (#515). A kept draft stays as typed unless
+        // another writer changed the set meanwhile: then the set leads it (I2). Text typed before the user saw any
+        // saved tags follows them, so a Save never drops tags it never showed.
+        const draft = keepDraft || typed ? state.manual : null;
+        const changed = keepDraft && value.tag_revision !== seenRevision;
+        const unseen = draft !== null && !keepDraft;
         const reason = generate.supported ? '' : (generate.reason || '');
         set({
             phase: 'ready',
             image: {filename: value.filename, title: value.title, width, height, mediaType, previewUrl,
                 dimensions: width && height ? `${width} × ${height}` : null},
             savedTags: value.tags.slice(), tagRevision: value.tag_revision,
-            manual: draft === null ? value.tags.join(', ') : changed ? mergeCurrent(value.tags, draft) : draft,
+            manual: draft === null ? value.tags.join(', ') : changed || unseen ? mergeCurrent(value.tags, draft) : draft,
             draftRevision: state.draftRevision + 1,
             ...savedModel, options: {...defaults, ...chosen?.options}, optionsRevision: state.optionsRevision + 1,
             generateSupported: generate.supported, generateReason: generate.reason,
@@ -402,6 +410,7 @@ export function createImageTaggingController({api, target, source = 'images', on
     }
 
     function setManual(text) {
+        typed = true;
         set({manual: typeof text === 'string' ? text : ''});
     }