fix: post-merge audit follow-ups (idle-wait default, incremental re-runs, eval config, progress, alpha) (#119)

* fix(index): make wait_network_idle default source-aware, not blanket

PR #96 defaulted wait_network_idle=True for every index-build URL render.
The SPA blank-tile problem it solves is real only for arbitrary external
URLs (the `web` source). Applied uniformly, the default:

- silently disabled the turbo (fast_cdp) capture path for ALL renders —
  cdp.py forces the standard path when wait_network_idle is set — roughly
  halving capture throughput on turbo-capable Chrome;
- added a >=500ms (up to 12s) per-page idle floor to kiwix (localhost)
  and local file:// text renders, where assets are ready before `load`
  fires and the wait buys nothing (see the measurements referenced in
  docs/screenshot-throughput-optimization.md);
- contradicted the still-present design record in cdp.py's docstring
  that the idle wait is opt-in for batch renders.

Default it on only for `source.type == "web"`; an explicit
`ingest: {wait_network_idle: ...}` still always wins. Update the cdp.py
docstring to record the new split. Also copy the ingest config before
mutating it, so pop/setdefault no longer leak into DEFAULT_CONFIG.

* fix(index): detect stale tile dirs on incremental re-runs; harden manifest writes

article_id is the source-enumeration position index, and every render path
skipped an existing {idx}.png.tiles on bare existence. When the source set
changes between runs (a file added/removed/renamed), positions shift: the
skip reused another document's pixels and the stamp loop then overwrote its
article_id with the new position — silently pairing one document's tiles
with another document's metadata, while the shifted document was never
rendered at all. This is the same misalignment class the manifest stamp
was introduced to fix, resurfacing through the skip path.

- _needs_render(): reuse a tile dir only if its manifest parses and records
  the same source document (`source`, falling back to legacy `url`) as the
  doc now occupying that position; on mismatch or corruption, warn, remove
  the stale dir, and re-render. All four render paths (URL/text/PDF/image)
  now go through it. Legacy text dirs (whose recorded url is a tempfile
  path) re-render once and are stamped with their true source thereafter.
- Stamp loop: also record `source` (the doc's url or path) so future runs
  can verify identity; skip the rewrite when already stamped (no mtime
  churn); write via tmp file + os.replace so a crash mid-write can never
  truncate a manifest.
- chunk.py: a corrupt tiles.json now skips that article with a warning
  instead of raising an uncaught JSONDecodeError that failed the whole
  build through subprocess check=True.

Tests: tests/test_incremental_rerun.py exercises the real helpers (missing
dir, match, legacy url match, shifted position, corruption at render and
chunk stages).

The positional identity itself remains; making identity content/path-stable
(articles.json as an id-keyed map) is a follow-up design change.

* fix(eval): let the model registry's api_base/api_key actually take effect

--api-base defaulted to "http://localhost:8000/v1" and --api-key to
"dummy" — both truthy — so the `args.api_base if args.api_base else
model_config[...]` resolution NEVER fell through to the model registry.
The MiniMax entries added in #113 (and any registry endpoint) were dead
on arrival via the documented `run_bench.py --model MiniMax-M3` flow:
requests always went to localhost:8000 with key "dummy".

Default both flags to None so explicit flags still win, the registry
(including MINIMAX_API_BASE / MINIMAX_API_KEY env overrides) is used
otherwise, and the old localhost/dummy values remain as final fallbacks
for models with no registry endpoint. The --open-router / commonstack
branches already treat "dummy" as unset, so None passes them unchanged.

* fix(embed): keep chunk progress bar out of pool workers; restore loggable progress

The Chunking tqdm bar added in #108 ran inside process_shard, which
--tiles-dir mode fans out to a ProcessPoolExecutor with up to 96 workers —
that many concurrent bars share one stderr and trample each other. Give
process_shard a progress flag: the bar stays for the single-shard path
(what `pixelrag index build` invokes), pool workers disable it, and the
parent shows one shard-level bar over as_completed instead. Also replace
the nonsensical `disable=not all_article_dirs` guard.

#108 also removed the periodic "Embedded X/Y" log record because it broke
the tqdm display — but that record was the only progress that reached log
files in non-interactive runs (nohup/CI/redirected stderr). Restore it at
1/100 cadence, emitted only when stderr is not a TTY, so interactive runs
keep a clean bar and unattended runs stay observable after the fact.

* fix(index): composite transparent local images onto white, not black

The local-image render path (#95) saved tiles as JPEG via a bare
convert("RGB"), which maps fully-transparent pixels to their underlying
RGB values — black for typical chart/logo/diagram exports. Dark-on-
transparent images (the standard matplotlib / design-tool export, and
issue #67's own chart.png example) became dark-on-black garbage:
embedded as noise, unretrievable, unreadable when served.

Composite anything with an alpha channel (RGBA/LA/PA, or palette images
carrying transparency) onto a white background before dropping to RGB;
opaque images keep the direct convert.

Author: andylizf

embed/src/pixelrag_embed/chunk.py

@@ -85,7 +85,13 @@ def chunk_article(article_dir: str, dry_run: bool = False, force: bool = False)
         raw = f.read().strip()
     if not raw:
         return None
-    meta = json.loads(raw)
+    try:
+        meta = json.loads(raw)
+    except json.JSONDecodeError:
+        # A truncated manifest (crash mid-write) must not take down the whole
+        # shard/build — skip this article like other unreadable dirs.
+        logger.warning("Corrupt tiles.json in %s — skipping", article_dir)
+        return None
 
     tile_names = meta.get("tiles", [])
     if not tile_names:
@@ -287,6 +293,7 @@ def process_shard(
     dry_run: bool = False,
     force: bool = False,
     delete_tiles: bool = False,
+    progress: bool = True,
 ) -> dict:
     """Chunk all articles in a shard directory."""
     t0 = time.time()
@@ -315,9 +322,10 @@ def process_shard(
         if article_dir.is_dir() and article_dir.name.endswith(".png.tiles")
     ]
 
-    for article_dir in tqdm(
-        all_article_dirs, desc="Chunking", disable=not all_article_dirs
-    ):
+    # The bar is disabled when this runs inside a ProcessPoolExecutor worker
+    # (--tiles-dir mode): up to 96 concurrent bars would trample each other
+    # on one terminal. The parent shows a shard-level bar instead.
+    for article_dir in tqdm(all_article_dirs, desc="Chunking", disable=not progress):
         total_articles += 1
 
         result = chunk_article(str(article_dir), dry_run=dry_run, force=force)
@@ -423,11 +431,18 @@ def main():
     with ProcessPoolExecutor(max_workers=args.workers) as pool:
         futures = {
             pool.submit(
-                process_shard, sd, args.dry_run, args.force, args.delete_tiles
+                process_shard,
+                sd,
+                args.dry_run,
+                args.force,
+                args.delete_tiles,
+                progress=False,
             ): sd
             for sd in shard_dirs
         }
-        for fut in as_completed(futures):
+        for fut in tqdm(
+            as_completed(futures), total=len(futures), desc="Chunking shards"
+        ):
             sd = futures[fut]
             try:
                 r = fut.result()

embed/src/pixelrag_embed/embed_cpu.py

@@ -19,6 +19,7 @@
 import json
 import logging
 import os
+import sys
 from pathlib import Path
 
 import numpy as np
@@ -203,6 +204,12 @@ def embed_items(
             pooled = pooled / pooled.norm()
             embeddings[i] = pooled.cpu().numpy().astype(np.float16)
 
+        # tqdm covers interactive runs but never reaches log files; when
+        # stderr is not a TTY (nohup/CI/redirects) emit a periodic record so
+        # long embeds stay observable after the fact.
+        if (i + 1) % 100 == 0 and not sys.stderr.isatty():
+            logger.info("Embedded %d/%d", i + 1, len(items))
+
     return embeddings
 
 

eval/run_bench.py

@@ -983,7 +983,7 @@ async def run_async(args):
             if args.api_base
             else (model_config["api_base"] or "http://localhost:8000/v1")
         )
-        api_key = args.api_key if args.api_key else model_config["api_key"]
+        api_key = args.api_key if args.api_key else (model_config["api_key"] or "dummy")
         model = model_config["model"]
 
     # Generate output filename with model name if output is not explicitly set
@@ -1299,9 +1299,18 @@ def main():
 
     # API args
     parser.add_argument(
-        "--api-base", type=str, default="http://localhost:8000/v1", help="API base URL"
+        "--api-base",
+        type=str,
+        default=None,
+        help="API base URL (default: the model config's endpoint, "
+        "else http://localhost:8000/v1)",
+    )
+    parser.add_argument(
+        "--api-key",
+        type=str,
+        default=None,
+        help="API key (default: the model config's key, else 'dummy')",
     )
-    parser.add_argument("--api-key", type=str, default="dummy", help="API key")
     parser.add_argument(
         "--open-router",
         action="store_true",

index/src/pixelrag_index/pipelines.py

@@ -1,7 +1,10 @@
 """End-to-end pipeline: source -> ingest -> chunk -> embed -> build."""
 
 import argparse
+import json
 import logging
+import os
+import shutil
 import subprocess
 import sys
 from pathlib import Path
@@ -11,6 +14,45 @@
 logger = logging.getLogger("pixelrag-index")
 
 
+def _needs_render(tiles_dir: Path, idx: int, doc) -> bool:
+    """True if {idx}.png.tiles must be (re)rendered for doc.
+
+    An existing tile directory is reusable only if its manifest parses and
+    records the same source document (``source``, falling back to the legacy
+    ``url`` field) as the doc that now occupies position ``idx``. Position
+    indices shift whenever the source set changes between runs (a file added,
+    removed, or renamed) — reusing the directory then would silently pair one
+    document's pixels with another document's metadata. On mismatch the stale
+    directory is removed and re-rendered; corrupt manifests (e.g. a crash
+    mid-write) are likewise re-rendered instead of poisoning later stages.
+    """
+    tile_dir = tiles_dir / f"{idx}.png.tiles"
+    manifest_path = tile_dir / "tiles.json"
+    if not manifest_path.exists():
+        return True
+    expected = doc.url or doc.path
+    try:
+        manifest = json.loads(manifest_path.read_text())
+        recorded = manifest.get("source") or manifest.get("url")
+    except (json.JSONDecodeError, OSError):
+        logger.warning("  Corrupt manifest in %s — re-rendering", tile_dir.name)
+        recorded = None
+    if recorded == expected:
+        return False
+    if recorded is not None:
+        logger.warning(
+            "  %s was rendered from %r but position %d now holds %r — "
+            "re-rendering (source set changed between runs; use --force "
+            "for a clean rebuild)",
+            tile_dir.name,
+            recorded,
+            idx,
+            expected,
+        )
+    shutil.rmtree(tile_dir, ignore_errors=True)
+    return True
+
+
 def _department_of(article: dict, source_root: str) -> str:
     """Department = first sub-directory under the source root holding the file.
 
@@ -49,17 +91,23 @@ def build(config: dict, limit: int | None = None, force: bool = False) -> Path:
     output = Path(config.get("output", "./index"))
     tiles_dir = output / "tiles"
     embeddings_dir = output / "embeddings"
-    ingest_cfg = config.get("ingest", {})
-    # Default to waiting for network idle — most modern pages are JS-rendered
-    # SPAs that produce blank/incomplete tiles without this. Users can opt out
-    # with `ingest: {wait_network_idle: false}` in their pixelrag.yaml.
-    ingest_cfg.setdefault("wait_network_idle", True)
+    # Copy so the pop/setdefault below never mutate the caller's dict (or the
+    # module-level DEFAULT_CONFIG when the yaml has no `ingest:` section).
+    ingest_cfg = dict(config.get("ingest", {}))
+    # Wait for network idle by default only for the `web` source (arbitrary
+    # external URLs), where JS-rendered SPAs fetch content after `load` and
+    # would otherwise produce blank/incomplete tiles. Everything else — kiwix
+    # (localhost), local text docs (file://) — has its assets ready before
+    # `load` fires (see docs/screenshot-throughput-optimization.md), and the
+    # idle wait would cost >=500ms per page AND disqualify the turbo capture
+    # path (cdp.py forces the standard path when wait_network_idle is set).
+    # An explicit `ingest: {wait_network_idle: ...}` in pixelrag.yaml wins.
+    if config.get("source", {}).get("type") == "web":
+        ingest_cfg.setdefault("wait_network_idle", True)
     embed_cfg = config.get("embed", {})
     device = embed_cfg.get("device", "cpu")
 
     if force:
-        import shutil
-
         for d in (tiles_dir, embeddings_dir):
             if d.exists():
                 shutil.rmtree(d)
@@ -68,7 +116,6 @@ def build(config: dict, limit: int | None = None, force: bool = False) -> Path:
 
     # Stage 1: Render documents to tiles
     # Use sequential integer IDs as tile directory names so embed/serve can map them
-    import json
     from pixelrag_render.render import render_urls, render_pdf
 
     logger.info("Stage 1/4: Rendering %d documents to tiles...", len(docs))
@@ -102,9 +149,7 @@ def build(config: dict, limit: int | None = None, force: bool = False) -> Path:
     # Render URL batch — skip already-captured articles
     if url_docs:
         new_url_docs = [
-            (idx, d)
-            for idx, d in url_docs
-            if not (tiles_dir / f"{idx}.png.tiles" / "tiles.json").exists()
+            (idx, d) for idx, d in url_docs if _needs_render(tiles_dir, idx, d)
         ]
         if new_url_docs:
             urls = [d.url for _, d in new_url_docs]
@@ -164,7 +209,7 @@ def _repl(m: re.Match) -> str:
 
         with tempfile.TemporaryDirectory(prefix="pixelrag_text_") as tmp_dir:
             for idx, doc in text_docs:
-                if (tiles_dir / f"{idx}.png.tiles" / "tiles.json").exists():
+                if not _needs_render(tiles_dir, idx, doc):
                     continue
                 src_path = Path(doc.path)
                 content = src_path.read_text(errors="replace")
@@ -196,9 +241,8 @@ def _repl(m: re.Match) -> str:
     # Render PDFs — use idx as tile directory name (like URLs) so directory
     # names are always the numeric article_id.
     for idx, doc in pdf_docs:
-        out_dir = tiles_dir / f"{idx}.png.tiles"
-        if (out_dir / "tiles.json").exists():
-            continue  # already rendered on a previous run
+        if not _needs_render(tiles_dir, idx, doc):
+            continue  # already rendered for this same document
         try:
             render_pdf(doc.path, str(tiles_dir), stem=str(idx))
         except Exception as e:
@@ -213,12 +257,25 @@ def _repl(m: re.Match) -> str:
         _MAX_WIDTH = 4000  # cap large images to avoid VRAM pressure during embedding
 
         for idx, doc in image_docs:
-            tile_dir = tiles_dir / f"{idx}.png.tiles"
-            if (tile_dir / "tiles.json").exists():
+            if not _needs_render(tiles_dir, idx, doc):
                 continue
+            tile_dir = tiles_dir / f"{idx}.png.tiles"
             tile_dir.mkdir(parents=True, exist_ok=True)
             try:
-                img = PILImage.open(doc.path).convert("RGB")
+                img = PILImage.open(doc.path)
+                # JPEG has no alpha: composite transparent images onto white
+                # before dropping the channel. A bare convert("RGB") maps
+                # fully-transparent pixels to their underlying RGB — black for
+                # typical chart/logo exports — turning them into dark garbage.
+                if img.mode in ("RGBA", "LA", "PA") or (
+                    img.mode == "P" and "transparency" in img.info
+                ):
+                    rgba = img.convert("RGBA")
+                    background = PILImage.new("RGB", rgba.size, "white")
+                    background.paste(rgba, mask=rgba.getchannel("A"))
+                    img = background
+                else:
+                    img = img.convert("RGB")
                 # Resize if too wide
                 if img.width > _MAX_WIDTH:
                     ratio = _MAX_WIDTH / img.width
@@ -240,22 +297,34 @@ def _repl(m: re.Match) -> str:
                 logger.warning("  FAILED image %s: %s", doc.id, e)
         logger.info("  Rendered %d local images", len(image_docs))
 
-    # Write article_id into each tile directory's manifests so the embed
-    # pipeline reads it explicitly instead of guessing from the directory name.
-    # tiles.json always exists here; chunks.json exists only for PDFs (pdf.py
-    # writes it at render time, and chunk.py then skips those dirs). For every
-    # other source chunks.json is created by Stage 2's chunk.py, which
-    # propagates article_id from tiles.json. So write whichever exist now.
-    for idx, _ in url_docs + text_docs + pdf_docs + image_docs:
+    # Write article_id and the source identity into each tile directory's
+    # manifests, so the embed pipeline reads the id explicitly instead of
+    # guessing from the directory name, and so the next run's _needs_render
+    # can detect stale directories after the source set changes. tiles.json
+    # always exists here; chunks.json exists only for PDFs (pdf.py writes it
+    # at render time, and chunk.py then skips those dirs). For every other
+    # source chunks.json is created by Stage 2's chunk.py, which propagates
+    # article_id from tiles.json. So write whichever exist now.
+    for idx, doc in url_docs + text_docs + pdf_docs + image_docs:
+        identity = doc.url or doc.path
         for manifest_name in ("tiles.json", "chunks.json"):
             manifest_path = tiles_dir / f"{idx}.png.tiles" / manifest_name
-            if manifest_path.exists():
-                try:
-                    manifest = json.loads(manifest_path.read_text())
-                    manifest["article_id"] = idx
-                    manifest_path.write_text(json.dumps(manifest))
-                except (json.JSONDecodeError, OSError):
-                    pass
+            if not manifest_path.exists():
+                continue
+            try:
+                manifest = json.loads(manifest_path.read_text())
+            except (json.JSONDecodeError, OSError):
+                logger.warning("  Could not stamp unreadable %s", manifest_path)
+                continue
+            if manifest.get("article_id") == idx and manifest.get("source") == identity:
+                continue  # already stamped — skip the rewrite
+            manifest["article_id"] = idx
+            manifest["source"] = identity
+            # Atomic replace: a crash mid-write must not truncate the manifest
+            # (a corrupt tiles.json would otherwise poison every later stage).
+            tmp_path = manifest_path.with_name(manifest_name + ".tmp")
+            tmp_path.write_text(json.dumps(manifest))
+            os.replace(tmp_path, manifest_path)
 
     # Save articles.json for serve API — title + URL per article.
     # Use the pipeline's sequential *position index* (0, 1, 2, …) rather than

render/src/pixelrag_render/backends/cdp.py

@@ -199,8 +199,11 @@ def _readiness_expr(wait_network_idle: bool) -> str:
 
     When ``wait_network_idle`` is set, also waits (after load) until no new
     resource has been fetched for ``NET_QUIET_MS`` — for SPAs that fetch their
-    content *after* load. This costs a quiet window per page, so it is opt-in
-    (the pixelbrowse skill / single-page renders), not the batch default.
+    content *after* load. This costs a quiet window (>= NET_QUIET_MS, up to
+    LOAD_TIMEOUT_MS) per page and disqualifies the turbo capture path, so it
+    is off by default here; the pixelbrowse skill and the index pipeline's
+    `web` source (arbitrary external URLs) enable it, while kiwix/localhost
+    and file:// batch renders stay on the fast path.
 
     Returns an async-IIFE expression resolving to the page height to tile.
     """

tests/test_incremental_rerun.py

@@ -0,0 +1,66 @@
+"""Incremental re-run safety: stale tile dirs must be re-rendered, not relabeled.
+
+Position indices are assigned by source enumeration order, so they shift when
+the source set changes between runs (a file added, removed, or renamed).
+_needs_render must detect that an existing {idx}.png.tiles directory was
+rendered from a *different* document and re-render it instead of letting the
+stamp loop silently pair one document's pixels with another's metadata.
+"""
+
+import json
+
+from pixelrag_index.pipelines import _needs_render
+from pixelrag_index.sources.base import Document
+
+
+def _make_tile_dir(tiles_dir, idx, manifest):
+    tile_dir = tiles_dir / f"{idx}.png.tiles"
+    tile_dir.mkdir(parents=True)
+    (tile_dir / "tiles.json").write_text(json.dumps(manifest))
+    return tile_dir
+
+
+def test_missing_dir_needs_render(tmp_path):
+    doc = Document(id="a", path="/src/a.md")
+    assert _needs_render(tmp_path, 0, doc) is True
+
+
+def test_matching_source_is_reused(tmp_path):
+    _make_tile_dir(tmp_path, 0, {"source": "/src/a.md", "tiles": ["tile_0000.png"]})
+    doc = Document(id="a", path="/src/a.md")
+    assert _needs_render(tmp_path, 0, doc) is False
+
+
+def test_legacy_url_field_is_reused(tmp_path):
+    # Dirs stamped before the `source` field existed recorded the render URL.
+    _make_tile_dir(tmp_path, 0, {"url": "https://example.com/x", "tiles": []})
+    doc = Document(id="x", url="https://example.com/x")
+    assert _needs_render(tmp_path, 0, doc) is False
+
+
+def test_shifted_position_forces_rerender_and_removes_stale_dir(tmp_path):
+    # Built over [a, c] -> dir 1 holds c. User adds b: position 1 is now b.
+    stale = _make_tile_dir(tmp_path, 1, {"source": "/src/c.md", "tiles": []})
+    doc_b = Document(id="b", path="/src/b.md")
+    assert _needs_render(tmp_path, 1, doc_b) is True
+    assert not stale.exists(), "stale dir must be removed before re-render"
+
+
+def test_corrupt_manifest_forces_rerender(tmp_path):
+    tile_dir = tmp_path / "0.png.tiles"
+    tile_dir.mkdir()
+    (tile_dir / "tiles.json").write_text('{"source": "/src/a.md", "til')  # truncated
+    doc = Document(id="a", path="/src/a.md")
+    assert _needs_render(tmp_path, 0, doc) is True
+    assert not tile_dir.exists()
+
+
+def test_corrupt_manifest_skips_article_in_chunk_stage(tmp_path):
+    # A truncated tiles.json must not crash the chunk stage (it used to raise
+    # an uncaught JSONDecodeError and fail the whole build via check=True).
+    from pixelrag_embed.chunk import chunk_article
+
+    article_dir = tmp_path / "0.png.tiles"
+    article_dir.mkdir()
+    (article_dir / "tiles.json").write_text('{"tiles": ["tile_0000.png"')  # truncated
+    assert chunk_article(str(article_dir)) is None