feat(setup): progress bars, RichHandler logging, log-level cleanup, TMDB audit dump#52
Open
abhichandra21 wants to merge 9 commits into
Open
feat(setup): progress bars, RichHandler logging, log-level cleanup, TMDB audit dump#52abhichandra21 wants to merge 9 commits into
abhichandra21 wants to merge 9 commits into
Conversation
Replace the indeterminate console.status spinner with a rich Progress bar that shows count, percent, running cache-hit tally, elapsed time, and ETA. enrich_batch gains an optional on_progress callback so the progress UI stays in setup.py and the enricher remains framework-free.
The progress bar now shows the running cache-hit count, so the per-title log line is redundant. It also interleaves with the rich Progress/spinner render and breaks the terminal display under DEBUG logging.
This reverts commit a0337e1.
The stdout StreamHandler couldn't coordinate with rich.progress/status, so DEBUG/INFO log lines emitted during the enrichment step appended to the spinner's render line instead of breaking onto their own line. Switch the stream handler to RichHandler bound to a module-level Console(stderr=True) exported from recommender.log, and have setup.py and main.py import that same Console so Progress and the logger share live state. RichHandler pauses the active Progress while it writes, so log records render cleanly above the bar. Side effects: * Stream logs now go to stderr (matched what most CLIs do already). Three test_log.py capsys assertions updated from .out to .err. * Fixed a pre-existing %d-on-None TypeError in enricher.enrich() that was hidden by previous logger configuration ordering in the test suite.
Apply the same Progress widget to the other two long-running setup steps: * TMDB metadata fetch — was an indeterminate console.status spinner with a periodic "i/N processed" line. Replaced with an inline bar driven by the existing enumerate() index. * Taste profile build — was a spinner that gave no feedback for the full duration of a 10-batch LLM job. Added an on_batch_progress callback to taste_profile_builder.build() so setup can drive a bar that fills as each batch completes (cached batches advance the bar immediately). Factored the Progress construction into a small _progress_bar() helper so all three steps use the same column layout (spinner, label, bar, M/N, optional extra, elapsed, eta).
User-facing setup output was dominated by lines that just echo internal state — temp directories, extracted CSV paths, provider model IDs, raw row counts before transforms. Each ingestion module also re-announced the per-platform parse count immediately before setup's own "<platform>: ok N events" console line restated the same number more cleanly. Reclassified to DEBUG so a normal run shows only the meaningful human-readable summaries; --debug or the rotating file log still captures everything for diagnostics. Affected: * netflix/prime/disney "reading <temp path>" / "read N raw rows" * apple_tv "Extracting from", "Found", "N watch events parsed" * llm "Using <provider> (fast=..., reason=...)" for all three providers Left at INFO: watch_index dedup/cleanup counts (one-shot summaries with unique information not duplicated by console output).
_audit_cache_mismatches() previously printed the first 10 entries per category and dropped the rest on the floor. With several hundred mismatches it was impossible to triage them or to feed the full list to an LLM-driven override-generator. Now writes the complete audit (no truncation) to recommender/cache/logs/tmdb_audit.txt with one section per category: unmatched titles, content-type mismatches, title mismatches, year mismatches, runtime mismatches, and weak matches. Console output still truncates to 10 per category so the run summary stays readable. Added a new "unmatched titles (no TMDB ID)" section so titles that report_unmatched flags are also captured in the same file, giving a single triage surface.
Addresses two P1/P2 findings from review of #52: P1 — Tests could overwrite the real TMDB audit artifact. _audit_cache_mismatches() now accepts an optional audit_output_path that callers can override; default remains config.TMDB_AUDIT_PATH so the production caller is unchanged. The five direct tests in test_main.py pass tmp_path / "audit.txt". Belt-and-suspenders: a new tests/conftest.py autouse fixture also patches config.TMDB_AUDIT_PATH globally so any future test that runs the setup pipeline indirectly (e.g. via run_setup) cannot clobber the real artifact either. Without this, the audit-via-run_setup test in test_main.py was still mutating the real file even with the param fix applied to the direct callers. P2 — Stale findings could survive a clean rerun. The previous code only wrote the file when has_any was true. A clean refresh after a previous bad run would leave the prior audit text intact and misrepresent the current state. Always rewrite, with an explicit "No mismatches detected." marker when sections are empty.
Addresses P2 finding from review of #52: the rotating file log was hard-coded to INFO regardless of level_override, so messages demoted to DEBUG (provider init in llm.py, temp-path traces in the ingestion parsers) never made it to logs/app.log even when the operator ran with --debug. The PR description claimed otherwise; this commit makes the claim true. setup_logging now derives file_level from stream_level: DEBUG when explicitly requested, INFO otherwise (matching prior behavior for default and WARNING configurations). _configure_file_handler takes the target level as a parameter so the werkzeug logger stays at INFO (no DEBUG-level Werkzeug traffic worth persisting).
Owner
Author
|
All three review findings addressed in 2 commits:
Full suite: 370 passed. No new tests for the conftest fixture itself; the proof is in the cross-suite diff check shown above. |
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
1 participant
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.
Context
This is the same work that was in PR #47, which was merged then reverted (#50) because it bypassed the project's normal review flow. Reopening here for a proper review pass. Branch is identical to #47's HEAD at the time of merge.
Summary
Four user-facing improvements to
./recommend setup --refresh-data:recommender.log,setup.py, andmain.pyall share the same Console instance.--debugand the rotating file log still capture everything. One-shot operational summaries (dedup counts, cache cleanup) stay at INFO._audit_cache_mismatchesnow writes the complete audit (unmatched + content-type/title/year/runtime mismatches + weak matches; no truncation) torecommender/cache/logs/tmdb_audit.txt. Console still truncates to 10 per category for readability.tests/test_main.py:1018, 1044, 1136, 1162, 1187passtmp_pathascache_dirbut the new audit-dump code writes toconfig.TMDB_AUDIT_PATHunconditionally. Runningpytest tests/clobbers the real audit file with a near-empty one (this actually happened in the session that produced these commits — wiped a 400-line audit down to 15 lines).Two fixes available:
audit_output_pathparameter to_audit_cache_mismatches(defaultconfig.TMDB_AUDIT_PATH); update the five tests to passtmp_path / "audit.txt".config.TMDB_AUDIT_PATHin those five tests.I'd recommend resolving this before any reviewer runs the test suite.
Out of scope
This PR does not address the TMDB matching quality problem itself (see #51). The audit dump in this PR makes that problem visible — it doesn't fix it.
Files
recommender/setup.pyrecommender/enricher.pyon_progresscallback;%d→%sbug fix on Nonetmdb_idrecommender/taste_profile_builder.pyon_batch_progresscallbackrecommender/log.pyrecommender/main.pyrecommender/ingestion/{netflix,prime,apple_tv,disney}.pyrecommender/llm.pyconfig.pyTMDB_AUDIT_PATHconstanttests/test_log.py.out→.errafter the stderr moveNet: 13 files, +162 / −51.
Test plan
pytest tests/passes (370 tests)./recommend setup --refresh-dataand verifies progress bars + log renderingrecommender/cache/logs/tmdb_audit.txtcontains all six sections in fullRelated