feat(embed): add tqdm progress bar to chunk stage, clean up embed logging - #108
Merged
Conversation
…ging - chunk.py: add tqdm progress bar for article-level chunking so users see real-time progress during Stage 2 (e.g. "Chunking: 50%|█████ | 4/8") - embed_cpu.py: remove redundant "Embedded X/Y" logger.info every 10 items — tqdm already shows this and the log lines break the progress bar display Tested with 8 Wikipedia pages (64 chunks produced).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Author
|
Maybe embedd is bottleneck needing to add progress bar |
andylizf
added a commit
that referenced
this pull request
Jul 16, 2026
…uns, 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
logger.info("Embedded %d/%d")every 10 items — tqdm already shows this info and the log lines break the progress bar display.Closes #99.
Test plan