Inference: stage-aware retries (run manifest, supersede-on-retry) + visualization suite

[zachtheyek]

Summary

Stacked on #120 (feature/inference-energy-detection-perf), #122 (feature/training-retry-overhaul), and #125 (feature/pfb-bandpass-equalization). The PR base integration/pr08-base is coordinator scaffolding that merges those three chains; it will be rebased onto master as the parents merge — review only the commits unique to this branch.

Implements inference-side stage-aware retries plus the inference visualization suite. Closes #124.

Retry overhaul

• apply_saved_config save_tag clobber fixed: the checkpoint section of a saved training config is now skipped entirely when layering under CLI flags — an inference run can no longer masquerade under the training run's tag (regression test; confirmed live on blpc3: the run logged under its datetime tag despite --config-path config_test_v17.json).
• New inference_cadences run manifest (schema v2 via the PRAGMA user_version mechanism from Training fault tolerance: stage-based resume, run manifest, DB supersede semantics #122; new tables need no ALTER step — the CREATE TABLE IF NOT EXISTS in _init_database covers old and new databases, the version stamp records the change): one row per (cadence, stage transition) — preprocessed when the stamp .npy lands (written by _process_cadence, never re-written on resume), a superseding inferred row on inference completion carrying the aggregate stats (n_stamps, n_candidates, JSON confidence-quantile summary from the full P(true) vector, stage duration_s), and failed when a cadence's inference stage dies. Aggregates in the manifest mean run-level artifacts don't depend on the positives-only inference_results table. Writer-queue writes, whitelisted query_inference_cadences, mark_superseded support (npy_path filter), (tag, npy_path, status) index.
• Stage-aware resume in the streaming loop: a live inferred row for (tag, npy_path) skips the cadence entirely (stored aggregates fold into the totals; when nothing is pending the models aren't even loaded); preprocessing artifacts only → straight to inference via the existing .npy skip; else preprocess.
• Supersede-on-retry: mark_superseded(inference_results, tag, npy_path=...) runs before each cadence's fresh rows land, so partial positives from a dead attempt can't mix in; the manifest follows new-row-plus-supersede on completion.
• Inference-stage failure containment (mirroring preprocessing's): log → failed manifest row → continue; the pass raises at the end so inference_command's retry loop (bounded by --max-retries) re-attempts — and the manifest skip means only failed cadences re-run.
• The obsolete cross-attempt cadence_data cache on the legacy --test-files path is dropped, and run_inference computes predict_proba once (returning P(true)/predictions/latents as per-cadence transients) instead of a second RF pass.

Visualization suite (src/aetherscan/inference_viz.py)

Nine deliverables, each saved under plots/inference/{save_tag}/ + Slack upload (train.py's pattern), all individually exception-guarded (_viz_safe) — a plot bug can never kill a science run. Strictly the OO Figure API, never pyplot (background threads exist; the global registry is not thread-safe).

1. ed_stat_distributions — all-window D'Agostino k² histogram (log-log, per-ON-file overlay, threshold line). The vectorized ED workers now also return a fixed log-spaced summary histogram of every finite window statistic (a few numpy ops per channel), accumulated per ON file into each cadence's metadata JSON.
2. ed_hit_spectrum — hit density vs frequency (MHz), pre- vs post-dedup (raw/merged hit frequency lists added to metadata).
3. bandpass_flattening — raw vs flattened integrated spectra with the removed model (scaled PFB response H or spline fit) overlaid; formalizes PFB static passband equalization as default bandpass flattener; tag-scoped preprocess output dir #125's opt-in debug artifact as a standard per-run figure (reuses the preprocessor's channel-read/flattener helpers so it can't drift from what detection does).
4. stamp_gallery — top-K stamps by statistic (--stamp-gallery-top-k, default 12) as the 6-obs ON/OFF waterfall grids scientists inspect; overlap-search offset copies collapsed via their shared statistic (median-start representative).
5. preproc_funnel — per cadence: raw hits → merged → stamps (+overlap) → snippets inferred, with per-cadence stamp storage annotated.
6. confidence_distribution — P(true) histogram over all snippets (log-y, threshold line, per-cadence overlay when ≤ 10 cadences), from fixed-bin histograms folded per cadence (memory stays O(#cadences)).
7. candidate_gallery + per-candidate candidate_{i}_{tag}.png — implements the long-standing plot_candidate() stub: 6-panel cadence waterfall, 48-dim latent bar chart (colored per observation), confidence/frequency/target/session/band/tstart annotations. Sourced from inference_results (covers resume-skipped cadences); per-candidate figures capped by --max-candidate-plots (default 50).
8. inference_latent_projection — this run's cadence-level latent features projected through the persisted training cadence-level UMAP (located via the training config JSON's model_path + save_tag + umap sweep values; skips gracefully with a log when absent), over the UMAP's training embedding as a faint backdrop; candidates starred. Latent pool bounded (20k rows, candidates always kept).
9. inference_summary — table card: cadence/snippet/candidate counts, resumed-cadence count, raw/merged hits, stamp storage, per-stage durations + throughput from the manifest, per-target/band candidate counts.

Config: inference_viz_enabled (default on) / stamp_gallery_top_k / max_candidate_plots + --inference-viz|--no-inference-viz, --stamp-gallery-top-k, --max-candidate-plots (validation, to_dict(), README CLI Reference regenerated via PYTHONPATH=src python utils/print_cli_help.py all).

Cluster validation — kill/retry + figures (blpc3, subset_test.csv = 2 cadences, dummy test_v17 model, --max-retries 1)

Run 1 (tag 20260711_224251): fresh run; the real cluster DB migrated version 1 → 2 on open. Cadence 1 (DDO210) preprocessed (790 s, 73,004 stamps) and inferred (102.7 s); cadence 2 (NGC7454) preprocessed (336.6 s, 21,663 stamps). A remote watcher SIGKILLed the whole process tree 10 s into cadence 2's inference (trigger: the second "Running inference on" log line).

Run 2 (identical command, same tag):

• Cadence ('DDO210', ...): already inferred under tag 20260711_224251 (73004 snippets, 0 candidate(s)); skipping → Streaming inference over 1 cadence(s) (1 already inferred, resumed from manifest) — cadence 1 skipped entirely (no ED, no load, no encode).
• Cadence 2 skipped ED via its existing .npy and went straight to inference (32.0 s), then the full viz suite rendered and the run completed successfully. Retry wall time ~3 m 14 s vs ~21 min for the original pass.

DB assertions after run 2 (sqlite):

• inference_cadences for the tag: exactly one live inferred row per cadence; both preprocessed rows superseded (preprocessed|1 / inferred|0 × 2).
• No duplicate live inference_results rows: GROUP BY npy_path, snippet_index HAVING COUNT(*) > 1 with superseded = 0 returns nothing.
• Confidence summaries stored on both inferred rows, e.g. DDO210: {"n": 73004, "threshold": 0.99, "n_above_threshold": 0, "mean": 0.0517, "max": 0.915, "quantiles": {"p01": 0.0, ..., "p99": 0.461}}.

Figures (run 2): 8 of 9 rendered under plots/inference/20260711_224251/ and uploaded to Slack (#aetherscan-logs; zero upload-failure signatures in the log — upload success is silent by design, same as train.py's plots): ed_stat_distributions, ed_hit_spectrum, bandpass_flattening, stamp_gallery, preproc_funnel, confidence_distribution, inference_latent_projection (through the real persisted umap_cadence_nn5_md0.1_test_v17.joblib, 20,000 features), inference_summary. The candidate figures correctly logged Viz: no candidates recorded; skipping candidate plots — the dummy model's max P(true) is 0.918, below the 0.99 threshold (0 candidates, consistent with #125's runs).

Run 3 (bonus, fresh tag 20260711_231234, --classification-threshold 0.8, --preprocess-output-dir pointing at run 1's directory — the documented reuse escape hatch): exercised the candidate path on-cluster. 109 candidates written (26 DDO210 + 83 NGC7454), 109 live inference_results rows, and all nine deliverables rendered under plots/inference/20260711_231234/: the eight run-2 figures plus candidate_gallery and exactly 50 per-candidate figures (Viz: rendered 50 of 109 candidate figures (--max-candidate-plots cap)), zero Slack upload failures. End-to-end ~6.5 min with ED skipped via the reused directory.

Local verification

• PYTHONPATH=src pytest -m "not gpu and not cluster" -q → 402 passed, 3 skipped (integration), 2 deselected (arm64 venv, Python 3.12, TF 2.17.1), after rebasing onto the refreshed integration/pr08-base (8e42ce5, incl. PFB static passband equalization as default bandpass flattener; tag-scoped preprocess output dir #125's PFB sidecar-cache memory fix).
• New unit tests: manifest write/query round-trip + status filters + supersede-on-retry flow + npy_path scoping; v0→v2 migration (old DB gains the table; idempotent across reopens); streaming state machine with the GPU pipeline stubbed (fresh-run manifest writes, skip-on-retry without model load, failed-cadence containment + solo re-attempt, stale-positive supersede, artifact-only resume, no-stamps non-retryable); summarize_confidences math (quantiles, strict threshold boundary, JSON round-trip); apply_saved_config checkpoint-skip regression; ED-worker histogram invariants; viz-flag routing + validation; and smoke tests for every figure (files exist, non-empty) including graceful-skip paths and a suite-entry test that survives fully broken records.
• ruff check / ruff format / all pre-commit hooks pass; all commits GPG-signed.

AI disclosure

Implemented end-to-end with Claude Code (model: Claude Fable 5): design-from-spec, implementation, unit tests, cluster kill/retry validation, and this PR description. Human oversight: repo owner reviews/merges.

🤖 Generated with Claude Code

[claude]

Claude finished @zachtheyek's task in 2m 36s —— View job

---

PR Review: Inference stage-aware retries + visualization suite

• Fetch base branch and read PR diff
• Review db/db.py — manifest schema, queries, migration
• Review main.py — streaming loop, retry logic, resume
• Review inference_viz.py — visualization suite
• Review inference.py, preprocessing.py, config.py, cli.py
• Review test coverage
• Post review with findings

---

Overall assessment

This is a well-structured, thoroughly tested PR. The retry overhaul and visualization suite are cleanly separated, the manifest-driven resume logic is sound, and the test coverage is comprehensive. The cluster validation evidence (kill/retry across 3 runs) demonstrates the correctness of the stage-aware resume mechanism convincingly. No blocking issues found — the inline comments are suggestions and observations, not required changes.

Architecture & design

Retry/resume mechanism — The manifest-driven approach (one row per (cadence, stage transition), supersede-on-retry before fresh rows land) is a solid design. The supersede ordering in _infer_cadence (L282–L359 of main.py) correctly serializes through the writer queue's FIFO: mark_superseded(inference_results) → write fresh results → mark_superseded(inference_cadences) → write the inferred manifest row. This prevents partial-positive contamination from dead attempts.

Visualization suite — The _viz_safe guard pattern is the right call for science-first code — a broken plot can never kill a run. Strict OO Figure API usage (no pyplot) is correct given the background threads. The nine figures cover the inspection surface area well.

apply_saved_config checkpoint-skip fix — Good catch on the save_tag clobber. The regression test (test_checkpoint_section_is_never_applied) nails it down properly.

Specific findings (inline comments posted)

1. inference_viz.py:206 — Figures are created but never explicitly cleared/closed. GC handles it, but for a nine-figure suite against large catalogs, eagerly releasing the artist tree after savefig would reduce the transient memory peak. Consider fig.clear() in _save_and_upload.

2. main.py:437 — DataPreprocessor._cadence_metadata_path is accessed from outside the class (private by convention). Consider promoting it or adding metadata_path to the work unit.

3. main.py:496-502 — The failed manifest row uses cadence_result.n_hits (raw hit count from ED) for n_stamps, which is semantically inconsistent with the preprocessed and inferred rows that use the actual stamp count. Not a correctness issue (the field is informational on failed rows), but could confuse manifest inspection.

4. inference_viz.py:339-344 / preprocessing.py:1618-1637 — Hit frequency lists are accumulated raw (all floats in memory) for the hit spectrum, unlike the ED stat histograms which are pre-binned. At the current scale this is fine, but it's an asymmetry worth noting for future large-catalog runs.

Code quality

• DB schema changes are well-managed: CREATE TABLE IF NOT EXISTS + _migrate_schema version gating means old databases gain the table seamlessly, and the tests verify this (test_old_schema_gains_inference_cadences_table).
• The NonRetryableInferenceError distinction (empty catalog vs. transient failure) is a good pattern — prevents burning retry attempts on permanent conditions.
• summarize_confidences correctly uses strict > for the threshold boundary, matching the RF prediction rule (proba > threshold), and the test (test_threshold_boundary_is_strict) locks this down.
• CLI validation for the new flags (stamp_gallery_top_k >= 1, max_candidate_plots >= 0) is correctly placed in collect_validation_errors with proper fix_kind annotations.

Test coverage

Strong coverage across all new code paths:

• DB: manifest write/query round-trip, status/npy_path filters, supersede-on-retry flow, npy_path scoping, v0→v2 migration, SQL injection rejection.
• Inference: summarize_confidences math, threshold boundary semantics, JSON round-trip, empty-vector guard.
• Viz: smoke tests for all nine figures (files exist + non-empty), collector budget/histogram tests, overlap-offset dedup in stamp selection, graceful-skip paths, and the critical test_render_survives_broken_records test.
• CLI: validation bounds, flag routing, omitted-flags defaults, and the apply_saved_config checkpoint-skip regression.
• Main: the streaming state machine tests (referenced in the PR description) cover fresh-run writes, skip-on-retry, failure containment, and artifact-only resume.

Security

No concerns. The new inference_cadences table uses parameterized queries throughout, column names are whitelisted via _INFERENCE_CADENCES_COLUMNS, and mark_superseded's table name is validated against _SUPERSEDE_TABLES before interpolation. The f-string in _execute_mark_superseded is safe because table has already been checked against a frozen set of known table names.

Performance

The pre-binning strategy for ED stat histograms (a few numpy ops per channel in the worker, combined by addition in the parent) is well-designed and avoids transferring large arrays. The latent reservoir sampling (_reservoir_add with a 20k-point budget, candidates always kept) keeps memory bounded regardless of catalog size. The mmap-based stamp loading for the gallery figures is appropriate.

[claude]

Contribution check: This PR mentions Closes #124 in the body, but no formal issue link is detected — likely because the PR targets integration/pr08-base rather than the default branch (GitHub only auto-closes issues when a PR merges into the default branch). Per CONTRIBUTING.md, please formally link the issue via the "Development" sidebar so the connection is tracked regardless of base branch. Once linked, the needs-issue label can be removed.

[zachtheyek]

Review round applied in 505cabb — thanks for the careful pass:

• Eager figure cleanup: _save_and_upload now calls fig.clear() right after savefig, releasing artists/render buffers immediately instead of at GC (matters for dense stamp/candidate galleries).
• Metadata path access: cadence_metadata_path promoted to a public DataPreprocessor method; main.py and the tests no longer reach into a _-prefixed helper.
• failed-row n_stamps: turned out to be already consistent — CadenceResult.n_hits is the stamp-row count on both construction paths (the raw ED hit count only lives in metadata as n_raw_hits); the historical field name was the trap. Documented at the field definition and the write site.
• Hit-frequency accumulation: # NOTE:s added at both the metadata write and the viz accumulation site naming the asymmetry with the pre-binned ED stat histograms and the pre-binning fix if catalogs grow.
• The mmap-handle observation needed no change (sequential calls, refcount-closed per call) — reasoning left on the thread.

Affected tests + full pre-commit pass; all inline threads answered individually.

[zachtheyek]

Re: contribution check — this is a stacked PR: its base is the coordinator-maintained integration/pr08-base scaffolding (merging #120 + #122 + #125), and it will be retargeted/rebased onto master as those parents merge. The Closes #124 keyword in the body will bind and auto-close once the PR targets the default branch; until then, linking #124 via the Development sidebar is the one-click manual option for tracking (maintainer action — the sidebar isn't writable via the API for this repo setup).

[zachtheyek]

Fixed a layout collision in the per-candidate inference plot (plot_candidate in inference_viz.py).

Bug: the right column split the shared n_obs x 2 gridspec into a bar chart (top) and a metadata text panel (bottom). The bar chart's x-axis label ("latent dimension (colored per observation)") sat right at that boundary and rendered directly on top of the first metadata line ("confidence: ..."), making both illegible.

Fix: the right column now uses its own nested 2x1 sub-gridspec with a dedicated vertical gap (hspace) between the bar chart and a taller text panel, and the metadata is anchored at the top of that panel. The axis label and all ~10 provenance lines now have clear separation, and the layout scales with the number of metadata lines. Still uses the OO Figure API (no pyplot), so it's safe on background threads. All existing content is preserved (every metadata field, per-observation color coding, the P(true) title).

Verification: rendered plot_candidate's layout against synthetic inputs (fake cadence stamp, 48-dim latent, full provenance dict) with numpy+matplotlib and visually confirmed the PNG: the x-axis label and metadata block no longer overlap, and both are legible. Confirmed the pre-fix layout reproduced the exact collision for contrast. pre-commit (ruff + ruff-format + hooks) passes on the changed file.

Verified render by Claude Code.

[zachtheyek]

@claude please perform a second-pass code review of this PR. Re-review the current diff for any remaining correctness bugs, security issues, or significant quality problems, and post your findings. If the diff is clean, please say so explicitly. (Final verification round before merge.)

[claude]

Bug: preprocessing duration will always be 0 for cadences that completed inference.

query_inference_cadences(tag=tag) defaults to include_superseded=False. But _infer_cadence (main.py) calls mark_superseded("inference_cadences", tag, npy_path=...) before writing the 'inferred' row — this supersedes the 'preprocessed' row too (it supersedes ALL prior rows for that (tag, npy_path)). So by the time this summary runs, every successfully-inferred cadence's 'preprocessed' row has superseded=1 and is invisible to this query.

The result: preproc_duration will only accumulate durations from cadences that failed inference (their 'preprocessed' rows are never superseded). For a fully successful run, preprocessing time shows 0.0 s.

Fix: pass include_superseded=True here so superseded 'preprocessed' rows contribute their durations:

Suggested change

	embedding[non_candidates, 0],
	embedding[non_candidates, 1],
	s=6,
	c="tab:blue",
	alpha=0.5,
	linewidths=0,
	label=f"snippets ({int(non_candidates.sum()):,})",
	rasterized=True,
	for row in db.query_inference_cadences(tag=tag, include_superseded=True):

[zachtheyek]

Fixed in 293ef96. Passed include_superseded=True on the query_inference_cadences(tag=tag) call so the superseded 'preprocessed' rows are visible to the summary again.

Verified this does not introduce double-counting: the aggregation loop already keys off status ('preprocessed' -> preproc_duration, 'inferred' -> inference_duration), so a cadence's superseded 'preprocessed' row and its live 'inferred' row land in separate buckets, each summed exactly once. A single row carries exactly one status and one duration_s, so nothing is counted twice. Added a NOTE at the query explaining why include_superseded is required here (contrary to most callers, which rely on the default).

Added a regression test test_inference_summary_counts_superseded_preprocessing that reproduces the post-success manifest state (a 'preprocessed' row superseded by its 'inferred' row, in the _infer_cadence order) and asserts the rendered preprocessing time is the real non-zero value (12.0 s), summed once — not 0.0 s (the bug) and not 24.0 s (a double-count). Confirmed it fails against the pre-fix code with exactly ['0.0', 's'] and passes after the one-token fix.

[claude]

Second-pass code review — PR #129

Performed a thorough re-review of the current diff: config/CLI, DB schema + migration, inference.py changes, the full inference_viz.py module, the streaming loop refactor in main.py, preprocessing changes, and all new unit tests.

Finding: one correctness bug

plot_inference_summary reports preprocessing duration as 0 on successful runs (inline comment). _infer_cadence supersedes the 'preprocessed' manifest row when writing the 'inferred' row, but the summary queries with include_superseded=False (the default). Fix is a one-token change: include_superseded=True on that query.

Everything else is clean

Specifically verified:

• DB schema v2 migration: CREATE TABLE IF NOT EXISTS before _migrate_schema is the right ordering; the v2 comment-only migration block is correct (new tables need no ALTER). The _SUPERSEDE_TABLES expansion, column whitelist, and bulk-insert statement are consistent.
• Supersede-on-retry ordering in _infer_cadence: the four-step sequence (supersede inference_results → write fresh results → supersede inference_cadences → write 'inferred' row) is correct and relies on FIFO queue ordering, which is guaranteed.
• Stage-aware resume: the skip logic correctly increments n_cadences for skipped cadences, so the n_cadences == 0 non-retryable check passes when all cadences resume. The pending list correctly gates model loading — no GPU memory wasted when everything is already inferred.
• RF predict_proba refactor in run_inference: probas[:, 1] extracts P(true) correctly; the threshold comparison uses strict > matching predict_verbose's semantics; confidence_scores via np.where preserves the same contract.
• summarize_confidences: math is correct, JSON-serializable (all numpy scalars cast to Python floats/ints), strict > threshold matching the RF decision boundary, ValueError on empty input.
• apply_saved_config checkpoint skip: correct — prevents the most damaging cross-run contamination (save_tag clobber). Regression test covers all four checkpoint fields.
• Failure containment: per-cadence inference failures log + record 'failed' + continue; the post-loop RuntimeError raise triggers the retry loop; the retry pass skips completed cadences via their manifest rows. The NonRetryableInferenceError paths are preserved.
• Viz suite safety: _viz_safe wraps every figure; OO Figure API throughout (no pyplot); fig.clear() after save frees memory; all test smoke tests confirm files exist and are non-empty.
• ED worker histogram: fixed log-spaced bins, np.clip prevents silent drops, per-ON-file accumulation shape is validated in the viz by checking cadence_edges.shape.
• InferenceVizCollector: memory stays bounded — confidence vectors folded into fixed-bin histograms immediately, latent features subsampled against _MAX_LATENT_POINTS with candidates always kept. prepare_latent_features reshapes to snippet-level before indexing, consistent with the is_candidate mask.
• Validation: stamp_gallery_top_k >= 1 and max_candidate_plots >= 0 bounds are correct.
• Tests: comprehensive coverage of the manifest state machine, migration, confidence math, viz smoke, and the streaming resume flow.

No security issues, no other correctness bugs, no significant quality problems beyond the one inline comment above. The diff is ready to merge after that one-line fix.

[zachtheyek]

@claude the finding from your previous review has been addressed in a new commit. Please re-review the current diff once more and confirm whether it is now clean, or report any remaining issues.

[claude]

Re-reviewed the current diff. The finding from my previous review (preprocessing duration always reading 0.0 s on a fully successful run because superseded 'preprocessed' rows were invisible to the default query_inference_cadences query) has been correctly addressed:

• plot_inference_summary now passes include_superseded=True (line 840 of inference_viz.py), with a thorough explanatory comment documenting the reasoning.
• A dedicated regression test (test_inference_summary_counts_superseded_preprocessing) directly asserts the fix: it reproduces the post-success manifest state (a superseded 'preprocessed' row + a live 'inferred' row), renders the summary, and verifies that preprocessing time reads 12.0 s (not 0.0) and inference time reads 3.0 s — confirming both that the superseded row contributes and that the per-status summing prevents double-counting.

No new issues found. The diff is clean — well-structured code, comprehensive test coverage for the new features, and all my earlier informational comments (memory/mmap, pre-binning asymmetry, private API usage) remain valid observations that don't need changes now.

🤖 Generated with Claude Code