feat(DATA-002): repair dirty candle data during the prefetch

[DoRmAmMu1997]

Why

DATA-001 gave the app eyes but no hands. validate_candles runs at the loader boundary during a scan and everything it can do is defensive: quarantine the symbol, log the codes, fold counts into scan_runs.data_quality_json. Nothing ever repaired the cached parquet, and the python app.py prefetch topped candles up without ever validating what it had just written.

The consequence is silent and permanent: a corrupt cache file stays corrupt forever, and its symbol is dropped from every scan without the operator noticing.

Measured against a real 577-file cache on 2026-08-17, no file was clean:

Code	Severity	Symbols	What the rows actually showed
STALE_LATEST_CANDLE	warn	577	570 stopped at one date; 7 stragglers were weeks further behind
DUPLICATE_DATE	fatal	18	Two distinct sub-classes needing opposite fixes
CALENDAR_DATE_GAP	warn	7	28–63 day holes in 2017-era illiquid counters; looks like genuine history
SUSPICIOUS_OVERNIGHT_PRICE_GAP	warn	6	Mostly a symptom of the duplicate problem
OPEN/CLOSE_OUTSIDE_RANGE	fatal	1	One bar with low == high but open below it — killing all 2,477 days

19 symbols were fatally dirty and being dropped from every scan.

What this does

Adds a repair pass that runs at the end of the prefetch — after the candles are topped up, before Streamlit boots — plus a standalone CLI.

The design principle: never invent a price. No interpolation, no clamping, no swapping a high and a low to make a bar "valid", no split adjustment. Each repair is one of three honest moves: remove redundancy, ask DhanHQ again, or drop what cannot be trusted — and only after the vendor has had its chance to re-supply it. Anything that could plausibly be real vendor data (a 50% overnight move that may be an unadjusted split; a multi-week gap in an illiquid small cap) is left alone and reported as NO_ACTION_VENDOR_DATA.

Structure mirrors the DATA-001 split: repair.py decides, cache_repair.py does. The planner is pure — no disk, no network, never mutates its input — which is what makes every rule testable with a three-row DataFrame instead of a broker session.

The duplicate-date split

The 18 duplicate symbols needed opposite treatments:

• Exact duplicates (13) — byte-identical rows for one date. De-duplicating costs zero trading days. One file carried 1,238 redundant rows.
• Conflicting duplicates (5) — the same date with different prices. One held 4,395 rows where ten years is ~2,470; another's series jumped 19.60 → 6.80 → 1.25 → 5.40. These are two price series merged by symbol renames, so the file is re-downloaded rather than edited.

The trap this avoids: one file had the same OHLC twice with volumes 164,890 then 2,245. The loader's existing drop_duplicates(keep="last") would keep the 2,245 row — the partial intraday snapshot, i.e. the wrong bar. A naive dedupe makes that file worse. Hence the volume-only tiebreak, which is opt-in, runs only after the vendor has had its chance, and never touches groups whose prices differ.

Verified against the real cache

Run against a copy of the live 577-file cache, with no network available:

Check	Result
Fatal symbols	19 → 5 (the 5 need a vendor refetch)
Rows removed	1,284, across 13 dedupes + 1 impossible bar
Trading days lost to a dedupe	0 (e.g. 3,714 → 2,476 rows, 2,476 → 2,476 days)
The impossible bar	Exactly one day dropped; that symbol's fatal findings cleared
Second consecutive run	No writes, no re-downloads (idempotent)
Stray .tmp files	None

Two guards came directly out of that verification:

• Refetch circuit breaker — the cache froze on 2026-07-24 because the Dhan access token expired (DH-901). An expired token fails identically for every symbol, so without a breaker one bad credential costs one wasted request per symbol on every launch.
• Cache-only short-circuit — with no credentials the offline half still runs, and symbols needing only the vendor report one clear reason instead of a wall of identical fetch errors.

Guardrails

• Drop budget measured in trading days, not rows — de-duplicating 1,238 redundant rows costs zero days and is always allowed; eating real history never is. Tripping it leaves the file untouched: a known-dirty file an operator can inspect beats a hollowed-out one.
• Write only on genuine improvement — a frame that got re-sorted while staying just as broken is churn (it bumps the mtime, invalidating chart caches). This makes "unrepairable means untouched" a real invariant.
• Atomic writes — temp file + os.replace, so an interrupted repair cannot corrupt the cache.
• Stale guard — the prefetch hands its per-symbol top-up statuses to the repair, so a symbol Dhan just answered is not asked again; standalone runs fall back to the existing .checked marker. Without this a full universe spends one pointless request per symbol.
• .repaired sidecar (7-day cooldown) — a symbol whose dirt lives in the vendor's own data would otherwise re-download its whole history every morning, forever.
• Repair failure never blocks boot — wrapped in try/except in the prefetch, same posture as the universe-refresh path.

Notable API addition

DailyDataLoader.fetch_window(...) — every other fetch path writes what it downloads straight to the symbol's parquet, which would truncate a ten-year file to the repaired window. This does the network half only (same pacing, DH-904 backoff, timeout) and lets the caller own the result.

Persistence

candle_repair_runs + migration 20260817data002, with a bounded, redacted, versioned receipt. A separate table rather than a scan_runs column because a repair is not a scan — it runs during the prefetch, before any screener executes. Admin health renders the newest row passively; there is deliberately no "Repair now" button, since a page refresh could then spend hundreds of broker requests.

Gates

All CI commands pass locally on the pushed tree: 1,863 passed, coverage 89.91% (floor 87%), plus ruff, mypy, compileall, bandit, pip-audit, and pre-commit validate-config. git diff origin/main HEAD -- constraints.txt pyproject.toml is empty (no dependency changes). Migration drift test updated for the new table in both hardcoded table sets.

A security review of the diff found no HIGH or MEDIUM issues: no path is built from input (the CLI's --symbol is a set-membership filter, never a filename), both dynamic error messages go through redact_text, the receipt takes the same normalize_secret_safe_json hop as audit metadata, and the health panel sits behind the existing admin guard.

Operator note

Your Dhan access token is expired, which is why the cache stopped advancing on 2026-07-24. The 5 remaining fatal symbols need a live re-download, so refresh the token (python Dependencies/dhan_token_setup.py) and the next python app.py should clear them.

🤖 Generated with Claude Code

[DoRmAmMu1997]

@codex can you review this?

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c76a5da9d8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Preserve improvements when the finding count stays flat

When a cache has multiple calendar gaps and Dhan fills only one, validate_candles still returns one CALENDAR_DATE_GAP finding before and after because the number of gaps is stored in affected_rows. Comparing only len(after.findings) therefore rejects the improvement, marks the symbol unrepairable, and discards the successfully recovered candles instead of writing a partial repair. Account for reduced affected rows or otherwise compare finding details, not only the number of finding objects.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Run offline repairs when Dhan credentials are unavailable

When DhanDataClient.from_env() raises for a missing or expired token, prefetch_data_assets() returns before reaching this new repair call. That skips repairs which require no client, such as exact de-duplication and dropping impossible bars, even though repair_universe explicitly supports a cache-only loader; affected fatal caches consequently remain quarantined from scans on every python app.py launch until credentials are restored or the standalone job is run.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Create the repair audit row before starting the pass

If the process crashes or repair_universe() raises during this call, execution never reaches persist_repair_summary(), so no candle_repair_runs row is created at all. This contradicts the new model and repository lifecycle that use finished_at IS NULL to expose an interrupted pass, leaving Admin health unable to distinguish a crashed repair from one that never started; create and commit the header before beginning the repair, then finish it afterward.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Include partial repairs in the still-dirty count

A partially_repaired outcome still has findings by definition and is included by CacheRepairSummary.needs_attention, but this persistence call records only symbols_unrepairable. Since the health page presents this database column as “Still dirty symbols,” a pass containing only partial repairs reports zero even though those symbols still require attention; include symbols_partially_repaired in the persisted count or expose a separate partial counter.

Useful? React with 👍 / 👎.

[DoRmAmMu1997]

Thanks — all four verified against the code. Three were real bugs and are fixed as suggested; the fourth I addressed differently, reasoning below. Pushed in bb72372.

1. Preserve improvements when the finding count stays flat — fixed, and this was the worst of the four.

Confirmed the premise: _calendar_gap_findings returns a single DataQualityFinding with the gap count in affected_rows. So filling one of three holes left len(findings) at 1 both sides, the repair was judged a no-op, and the candles Dhan had just supplied were thrown away.

Replaced the comparison with a lexicographic profile:

(fatal findings, total findings, total affected rows)

Fatal still dominates, so trading a fatal finding for a warning — dropping an impossible bar leaves a one-day hole, raising CALENDAR_DATE_GAP — is still accepted as progress. Added test_filling_one_of_several_gaps_counts_as_an_improvement (fails on the old code) plus test_improvement_still_rejects_a_repair_that_changes_nothing so the looser rule can't start accepting no-ops.

2. Run offline repairs when Dhan credentials are unavailable — fixed.

Correct: the except around DhanDataClient.from_env() returned before ever reaching the repair. The no-credentials path now runs repair_candle_cache_assets with an explicit cache-only loader, so de-duplication and impossible-bar drops still happen. That matters because those are exactly the repairs that unquarantine whole symbols, and they need no client at all.

3. Create the repair audit row before starting the pass — fixed.

The code contradicted its own docstrings, which promise finished_at IS NULL as the marker of an interrupted pass. Split into open_repair_run() (commits the header before the work) and persist_repair_summary(..., run_id=...) (stamps the counts). If the header can't be opened the finish path still creates a row, so a DB blip doesn't lose the receipt entirely; a dry run writes nothing.

4. Include partial repairs in the still-dirty count — addressed differently.

Agreed there's a real mismatch, but I think it's in the label, not the count, so conflating the two would trade one wrong signal for another.

Concretely, from the real cache: TVSHLTD went [CLOSE_OUTSIDE_RANGE, OPEN_OUTSIDE_RANGE, STALE_LATEST_CANDLE] → [STALE_LATEST_CANDLE]. Its fatal findings cleared, so it is scannable again — it just isn't perfectly clean. Counting that as "still dirty" would raise a false alarm about a symbol we just fixed; folding it into symbols_unrepairable would then hide the symbols nothing could be done for.

So instead:

• the metric is renamed to "Unrepairable symbols", which is exactly what the column holds;
• the partial count is surfaced separately, read from the receipt (symbols_partially_repaired was already persisted there), so those symbols stay visible without being mislabelled.

No migration change needed. Happy to add a dedicated column instead if you'd prefer partial counts queryable in SQL rather than only in the receipt JSON.

---

Gates on the branch: 1,870 tests pass at 89.72% coverage; ruff, bandit and compileall clean. mypy is clean under the pandas-stubs version this branch pins (2.3.3.260113, which is what CI installs) — I verified that explicitly, since my local environment has the 3.0 stubs from a separate dependency-bump PR.