fix(magazine): approve failed with unknown block ids after deleting or converting a region

The review selection now follows deletes, merges, splits and text-to-image
changes; approve sends only existing ids and no longer overwrites the image
list with a stale one; deleting a region removes it from articles. v1.38.1.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

CHANGELOG.md

@@ -7,6 +7,12 @@ Follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) · versioned wi
 
 ## [Unreleased]
 
+## [1.38.1] — 2026-09-21
+
+### Fixed
+- **Magazine: "Could not approve: Unknown block ids".** After deleting a region, merging it, or changing a text region into an image, the review page still had the old id selected and Approve sent it; worse, it sent a stale image list that dropped a region just turned into an image. The review selection now follows every such change (a converted region keeps its selected state under its new id), Approve only sends ids that still exist, and deleting a region also removes it from every article that listed it.
+- Keyboard focus in the review list now lands on the row that is focused when the list is redrawn, not the one focused when a change started.
+
 ## [1.38.0] — 2026-09-21
 
 ### Added

VERSION

@@ -1 +1 @@
-1.38.0
+1.38.1

routes/magazine.py

@@ -1037,6 +1037,17 @@ async def magazine_delete_region(doc_id: str, page: int, block_id: str):
         raise HTTPException(404, "Region not found")
     layout = mag.delete_region(layout, block_id)
     mag.save_layout(d, layout)
+    # Also drop it from articles: an article still listing a deleted id made Approve fail with
+    # "Unknown block ids".
+    articles = mag.load_articles(d)
+    changed = False
+    for article in articles:
+        for key in ("blocks", "images"):
+            if block_id in (article.get(key) or []):
+                article[key] = [x for x in article[key] if x != block_id]
+                changed = True
+    if changed:
+        mag.save_articles(d, articles)
     return {"ok": True}
 
 

static/index.html

@@ -27,12 +27,12 @@
 
   <!-- ── Core styles (local — no CDN dependency for first paint) ────────── -->
   <link rel="stylesheet" href="/static/vendor/mdi/materialdesignicons.min.css">
-<link rel="stylesheet" href="/static/style.css?v=1.38.0">
+<link rel="stylesheet" href="/static/style.css?v=1.38.1">
-<link rel="stylesheet" href="/static/css/resource-diagnostics.css?v=1.38.0">
+<link rel="stylesheet" href="/static/css/resource-diagnostics.css?v=1.38.1">
-<link rel="stylesheet" href="/static/css/background-jobs.css?v=1.38.0">
+<link rel="stylesheet" href="/static/css/background-jobs.css?v=1.38.1">
-<link rel="stylesheet" href="/static/css/voice-design.css?v=1.38.0">
+<link rel="stylesheet" href="/static/css/voice-design.css?v=1.38.1">
-<link rel="stylesheet" href="/static/css/rehearser.css?v=1.38.0">
+<link rel="stylesheet" href="/static/css/rehearser.css?v=1.38.1">
-<link rel="stylesheet" href="/static/css/audiobook.css?v=1.38.0">
+<link rel="stylesheet" href="/static/css/audiobook.css?v=1.38.1">
 
 
   <!-- ── Flag icons — non-blocking (loaded async, icons appear after JS) ── -->

static/js/magazine.js

@@ -701,9 +701,10 @@ async function magReviewChangeType(regionId, type, select) {
   if (select) select.disabled = true;
   say(fromImage && !toImage ? 'Reading the region as text…' : 'Changing region type…', 'info');
   try {
-    await api('/api/magazine/' + state.doc.id + '/pages/' + state.reviewPage + '/regions/' + encodeURIComponent(regionId), {
+    const r = await api('/api/magazine/' + state.doc.id + '/pages/' + state.reviewPage + '/regions/' + encodeURIComponent(regionId), {
       method: 'PATCH', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ type }),
     });
+    if (r && r.id && r.id !== regionId) magRemapSelection(regionId, r.id, toImage);
   } catch (e) {
     if (select) select.disabled = false;
     return say('Could not change the region type: ' + e.message, 'err');
@@ -1536,10 +1537,11 @@ function magRegionUrl(id) {
 }
 
 async function magReloadReview(message) {
-  // Keep a keyboard user's place: the row that had focus gets it back after the list is redrawn.
+  // Keep a keyboard user's place: the row focused when the redraw happens (not when the change
-  const had = document.activeElement?.closest?.('[data-block-row]')?.dataset.blockRow;
+  // started -- the user may have moved on meanwhile) gets focus back.
   state.layout = null;
   await ensureLayout();
+  const had = document.activeElement?.closest?.('[data-block-row]')?.dataset.blockRow;
   renderReview();
   if (had) document.querySelector('#mag-review [data-block-row="' + CSS.escape(had) + '"] input[type="checkbox"]')?.focus();
   if (message) say(message, 'ok');
@@ -1573,7 +1575,8 @@ function magRowEdit(id) {
         return magReloadReview('Text saved');
       }
       if (act === 'split') {
-        await api(magRegionUrl(id) + '/split', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ at: area.selectionStart }) });
+        const r = await api(magRegionUrl(id) + '/split', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ at: area.selectionStart }) });
+        if (r?.new_id) magSelectionAddAlongside(id, r.new_id);
         return magReloadReview('Region split in two');
       }
       magReloadReview();
@@ -1596,7 +1599,8 @@ async function magRowReocr(id) {
 
 async function magRowMerge(id) {
   try {
-    await api(magRegionUrl(id) + '/merge-next', { method: 'POST' });
+    const r = await api(magRegionUrl(id) + '/merge-next', { method: 'POST' });
+    if (r?.removed) magRemapSelection(r.removed, null, false);
   } catch (e) {
     return say('Could not merge: ' + e.message, 'err');
   }
@@ -1635,6 +1639,7 @@ async function magReviewDeleteRegion(blockId) {
     await api('/api/magazine/' + state.doc.id + '/pages/' + state.reviewPage + '/regions/' + encodeURIComponent(blockId), {
       method: 'DELETE',
     });
+    magRemapSelection(blockId, null, false);
   } catch (e) {
     return say('Could not delete region: ' + e.message, 'err');
   }
@@ -1846,10 +1851,35 @@ async function magReviewSubmitDraw() {
   renderReview();
 }
 
+// Keep the review's unsaved selection in step with a region change made on the server. Without this,
+// Approve sent ids that no longer existed ("Unknown block ids") after a delete, merge or type change,
+// and sent a stale image list that dropped a text box just turned into an image.
+function magRemapSelection(oldId, newId, newIsImage) {
+  const a = state.article;
+  if (!a) return;
+  if (!_magReviewChecked) _magReviewChecked = new Set(a.blocks || []);
+  const wasOn = _magReviewChecked.has(oldId) || (a.images || []).includes(oldId);
+  _magReviewChecked.delete(oldId);
+  a.blocks = (a.blocks || []).filter((id) => id !== oldId);
+  a.images = (a.images || []).filter((id) => id !== oldId);
+  if (!newId || !wasOn) return;
+  if (newIsImage) a.images = a.images.concat([newId]);
+  else { _magReviewChecked.add(newId); a.blocks = a.blocks.concat([newId]); }
+}
+
+// A region split in two: the new part is included wherever the original was.
+function magSelectionAddAlongside(existingId, newId) {
+  const a = state.article;
+  if (!a) return;
+  if (!_magReviewChecked) _magReviewChecked = new Set(a.blocks || []);
+  if (_magReviewChecked.has(existingId)) { _magReviewChecked.add(newId); a.blocks = (a.blocks || []).concat([newId]); }
+}
+
 function selectedBlockIds() {
-  if (_magReviewChecked) return Array.from(_magReviewChecked);
+  const exists = new Set((state.layout?.blocks || []).map((b) => b.id));
-  return Array.from(document.querySelectorAll('#mag-review input[data-block]:checked'))
+  const ids = _magReviewChecked ? Array.from(_magReviewChecked)
-    .map((el) => el.dataset.block);
+    : Array.from(document.querySelectorAll('#mag-review input[data-block]:checked')).map((el) => el.dataset.block);
+  return state.layout ? ids.filter((id) => exists.has(id)) : ids;
 }
 
 async function magReocrPage() {
@@ -1945,7 +1975,7 @@ async function magApprove() {
       headers: { 'Content-Type': 'application/json' },
       body: JSON.stringify({
         blocks,
-        images: a.images || [],
+        images: (a.images || []).filter((id) => !state.layout || (state.layout.images || []).some((img) => img.id === id)),
         status: 'approved',
         title: ($('mag-rev-title')?.value || a.title).trim(),
         author: ($('mag-rev-author')?.value || a.author).trim(),

tests/magazine.spec.js

@@ -545,3 +545,39 @@ test('the review list can be worked from the keyboard', async ({ page }) => {
   await page.keyboard.press('Enter');
   await expect(page.locator('[data-block-row="b3"]').getByRole('textbox', { name: 'Text of this region' })).toBeFocused();
 });
+
+test('approving after turning a ticked text box into an image sends the new image, not the old id', async ({ page }) => {
+  await mockBackend(page);
+  let converted = false;
+  await page.route(`**/api/magazine/${DOC_ID}/layout`, route => route.fulfill({ json: converted
+    ? { ...LAYOUT, blocks: LAYOUT.blocks.filter(b => b.id !== 'b3'), images: [{ id: 'p0003-manual-000', page: 3, bbox: [50, 150, 200, 300], filename: 'x.png', reading_order: 2 }] }
+    : LAYOUT }));
+  await page.route(`**/api/magazine/${DOC_ID}/pages/3/regions/b3`, route => {
+    converted = true;
+    return route.fulfill({ json: { ok: true, id: 'p0003-manual-000', type: 'image' } });
+  });
+  await openReview(page);
+  await expect(page.locator('#mag-review input[data-block="b3"]')).toBeChecked();
+  await page.locator('[data-block-row="b3"] select.mag-block-type-select').selectOption('image');
+  await expect(page.locator('[data-block-row="p0003-manual-000"]')).toBeVisible();
+  const request = page.waitForRequest(r => r.method() === 'PATCH' && r.url().includes('/articles/article-001'));
+  await page.getByRole('button', { name: 'Approve', exact: true }).click();
+  const body = (await request).postDataJSON();
+  expect(body.blocks).not.toContain('b3');
+  expect(body.images).toContain('p0003-manual-000');
+});
+
+test('approving after deleting a ticked box does not send its id', async ({ page }) => {
+  await mockBackend(page);
+  let deleted = false;
+  await page.route(`**/api/magazine/${DOC_ID}/layout`, route => route.fulfill({ json: deleted
+    ? { ...LAYOUT, blocks: LAYOUT.blocks.filter(b => b.id !== 'b3') } : LAYOUT }));
+  await page.route(`**/api/magazine/${DOC_ID}/pages/3/regions/b3`, route => { deleted = true; return route.fulfill({ json: { ok: true } }); });
+  await openReview(page);
+  await page.locator('[data-block-row="b3"]').getByRole('button', { name: /Delete region/ }).click();
+  await page.getByRole('alertdialog').getByRole('button', { name: 'Delete', exact: true }).click();
+  await expect(page.locator('[data-block-row="b3"]')).toHaveCount(0);
+  const request = page.waitForRequest(r => r.method() === 'PATCH' && r.url().includes('/articles/article-001'));
+  await page.getByRole('button', { name: 'Approve', exact: true }).click();
+  expect((await request).postDataJSON().blocks).not.toContain('b3');
+});

tests/test_magazine_routes.py

@@ -544,7 +544,8 @@ class DeleteRegionRouteTests(unittest.IsolatedAsyncioTestCase):
         layout = {"blocks": [{"id": "b1", "page": 1}], "images": []}
         with patch('routes.magazine._require_magazine', return_value=(None, {})), \
              patch('routes.magazine.mag.load_layout', return_value=layout), \
-             patch('routes.magazine.mag.save_layout') as save:
+             patch('routes.magazine.mag.save_layout') as save, \
+             patch('routes.magazine.mag.load_articles', return_value=[]):
             result = await magazine_delete_region('a' * 32, 1, 'b1')
         self.assertTrue(result['ok'])
         save.assert_called_once()
@@ -555,7 +556,8 @@ class DeleteRegionRouteTests(unittest.IsolatedAsyncioTestCase):
         layout = {"blocks": [], "images": [{"id": "img1", "page": 1}]}
         with patch('routes.magazine._require_magazine', return_value=(None, {})), \
              patch('routes.magazine.mag.load_layout', return_value=layout), \
-             patch('routes.magazine.mag.save_layout'):
+             patch('routes.magazine.mag.save_layout'), \
+             patch('routes.magazine.mag.load_articles', return_value=[]):
             result = await magazine_delete_region('a' * 32, 1, 'img1')
         self.assertTrue(result['ok'])
 
@@ -578,6 +580,23 @@ class DeleteRegionRouteTests(unittest.IsolatedAsyncioTestCase):
                 await magazine_delete_region('a' * 32, 1, 'b1')
             self.assertEqual(err.exception.status_code, 404)
 
+    async def test_a_deleted_region_leaves_every_article_that_listed_it(self):
+        """An article still listing a deleted id made Approve fail with "Unknown block ids"."""
+        from unittest.mock import patch
+        from routes.magazine import magazine_delete_region
+        layout = {"blocks": [{"id": "b1", "page": 1}, {"id": "b2", "page": 1}], "images": [{"id": "img", "page": 1}]}
+        articles = [{"id": "a1", "blocks": ["b1", "b2"], "images": ["img"]}, {"id": "a2", "blocks": ["b2"], "images": []}]
+        saved = {}
+        for rid, expect in (("b1", [["b2"], ["img"], ["b2"]]), ("img", [["b1", "b2"], [], ["b2"]])):
+            with self.subTest(rid=rid), \
+                 patch('routes.magazine._require_magazine', return_value=(None, {})), \
+                 patch('routes.magazine.mag.load_layout', return_value=layout), \
+                 patch('routes.magazine.mag.save_layout'), \
+                 patch('routes.magazine.mag.load_articles', return_value=[dict(a) for a in articles]), \
+                 patch('routes.magazine.mag.save_articles', side_effect=lambda _d, a: saved.__setitem__('a', a)):
+                await magazine_delete_region('a' * 32, 1, rid)
+                self.assertEqual([saved['a'][0]['blocks'], saved['a'][0]['images'], saved['a'][1]['blocks']], expect)
+
 
 class ChangeRegionTypeTests(unittest.TestCase):
     """Changing what a detected region is, from the review list."""